Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), lazy fetching has been controlled by the `GIT_NO_LAZY_FETCH` environment variable. This is currently an "all or nothing" boolean that is set to 'true' by default when calling `git upload-pack` for security reasons.

Recently the "promisor-remote" capability was added to protocol v2, allowing servers and clients to agree on the promisor remotes they can safely use.

This series leverages that capability to implement a pragmatic middle ground. By setting `GIT_NO_LAZY_FETCH` to 'fromAccepted', lazy fetching is allowed only when fetching from promisor remotes that are both advertised by the server and accepted by the client.

Note that using an environment variable for this is probably not the best from a usability perspective. An `upload-pack.allowLazyFetch` configuration variable would likely be better.

Unfortunately the `GIT_NO_LAZY_FETCH` environment variable is the way things currently work. It would be a much bigger and more invasive change to implement `upload-pack.allowLazyFetch` in a way that is compatible with `GIT_NO_LAZY_FETCH` which has to stay anyway for backward compatibility. Therefore, transitioning to a configuration variable is left for future work.

High level overview of the patches ==================================

Patch 1/3: A refactor which separates the fetching logic from the error handling and validation logic. This might also slightly increase performance if there are several promisor remotes.

Patch 2/3: A preparatory commit that transitions `GIT_NO_LAZY_FETCH` from a strict boolean check into an enum that can support multiple states.

Patch 3/3: Introduces the 'fromAccepted' option, taking advantage of the previous preparatory commits.

CI tests ========

They all pass, see:

https://github.com/chriscool/git/actions/runs/29078195030

Christian Couder (3):
  promisor-remote: factor out lazy_fetch_objects()
  promisor-remote: introduce enum allow_lazy_fetch
  promisor-remote: teach 'fromAccepted' to GIT_NO_LAZY_FETCH

 Documentation/git-upload-pack.adoc    |   5 ++
 Documentation/git.adoc                |   6 +-
 promisor-remote.c                     | 110 ++++++++++++++++++--------
 promisor-remote.h                     |  14 ++++
 setup.c                               |   5 +-
 t/t5710-promisor-remote-capability.sh |  49 ++++++++++++
 6 files changed, 154 insertions(+), 35 deletions(-)

In "promisor-remote.c:fetch_objects()", there is a check to disable lazy fetching when the `GIT_NO_LAZY_FETCH` environment variable is set. The fetch_objects() function is called once per promisor remote though. So the check might be performed more times than necessary.

Also promisor_remote_get_direct() mixes up the logic deciding which promisor remotes to try with the logic checking that the objects that could not be fetched are promisor objects.

Let's refactor the lazy fetching logic out of these two functions into a new lazy_fetch_objects() function. This will make it easier to extend the lazy fetching logic in following commits.

This is a pure refactoring with no intended behavior change. Two things shift in ways that are observably equivalent though:

  - the `GIT_NO_LAZY_FETCH` check is now performed once up front,
    instead of once per promisor remote, and

  - promisor_remote_init() is no longer called when lazy fetching
    is disabled, which is fine as nothing downstream of it, like
    is_promisor_object(), needs it in that case.

While at it, let's also convert try_promisor_remotes() to return 'bool' instead of 'int', as it just returns whether all the objects could be fetched, and document its return value.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 76 ++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

The `GIT_NO_LAZY_FETCH` environment variable is currently parsed as a Boolean, using git_env_bool(), in both "setup.c" and "promisor-remote.c".

In a following commit, we are going to allow a third value for this variable, on top of 'true' and 'false'.

To prepare for that, let's introduce an `enum allow_lazy_fetch` with the possible results of parsing the variable, along with a parse_allow_lazy_fetch_env() function to parse it, and let's use them everywhere the variable is parsed.

Note that, as before, an invalid value makes us die(), only the error message changes from "bad boolean environment value ..." to "bad environment value ...".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 24 +++++++++++++++++++++++-
 promisor-remote.h | 13 +++++++++++++
 setup.c           |  5 ++++-
 3 files changed, 40 insertions(+), 2 deletions(-)

The `GIT_NO_LAZY_FETCH` environment variable can be set to 'true' or 'false' to enable or disable lazy fetching. By default it is set to 'true' when calling `git upload-pack` to avoid security issues, see 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16).

Recently though, the "promisor-remote" capability was introduced into protocol v2, which allows a server to advertise some promisor remotes and clients to accept them or not.

When promisor remotes are advertised by the server and accepted by the client, it means that they are quite trusted. So the security risks which come from lazy fetching from them could be considered much more acceptable.

Let's introduce a 'fromAccepted' option on top of 'true' and 'false' for `GIT_NO_LAZY_FETCH` to allow lazy fetching only from accepted promisor remotes.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-upload-pack.adoc    |  5 +++
 Documentation/git.adoc                |  6 ++--
 promisor-remote.c                     | 14 +++++++-
 promisor-remote.h                     |  1 +
 t/t5710-promisor-remote-capability.sh | 49 +++++++++++++++++++++++++++
 5 files changed, 71 insertions(+), 4 deletions(-)

On 2026-07-10 at 08:51:34, Christian Couder wrote:

I don't think this is a good idea. We get a lot of reports on the security list involving various tooling that isn't within the scope of our threat model. This substantially increases the amount of code which is now subject to that threat model and therefore our security guarantees and I don't think we should do that as it stands, very especially while so much of our network-facing code is written in C.

The fetch code by default reads lots of configuration information from the repository, including remote settings and information and we really want absolutely none of that code running in the context of an untrusted repository.

On Fri, Jul 10, 2026 at 9:50 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

This small series doesn't change any defaults, especially GIT_NO_LAZY_FETCH is still set to 1 when calling `git upload-pack` by default. And the new option is more restrictive than the GIT_NO_LAZY_FETCH=0 option which already exists.

So I don't think it's fair to say that this _substantially increases_ the amount of code subject to some threat model.

I agree that client acceptance of some promisor remotes doesn't make the served repo trusted. It's a real concern, but I think it's addressable by different mechanisms. See below.

When a promisor remote has been accepted, it means both the client and the server trust it, so at least the promisor remote is not untrusted.

Now the main security issue on the server side is making sure the served repo itself is also trusted. And I agree that the operator of the server should decide and mark that trust, not the client.

I also agree that on GitLab/GitHub-style multi-tenant hosts most repositories shouldn't be marked as trusted.

However note that:

- The operator of the server is the only actor which can set
GIT_NO_LAZY_FETCH on the server (where it matters).
- In the case of corporate/self-hosted repos, the operator also
controls the repos.
- Different features could be developed (in future work) to improve on
the current state:
    - a way for lazy fetching to work without reading config files,
triggering hooks, or doing potentially sensitive things,
    - an explicit way for operators to mark trusted repos (like
perhaps a server-side config the operator sets per-repo),
    - operator-defined allow/deny rules, or maybe
    - some ways/scripts/commands to scan repos and check configuration
information, remote settings and everything potentially sensitive to
decide if a repo looks safe enough to allow lazy fetching or not.

I would be happy to hear opinions about those potential features or any other ways to address the issue.

So I agree that this series doesn't fix all the problems on the server side, but I think it's still valuable to be able to restrict lazy fetching to accepted promisor remotes.

Also I definitely agree that the current series should have better documentation about this, and I plan to improve on that in the v2 of this series.

Thanks for your insightful comments.

Recently the "promisor-remote" capability was added to protocol v2, allowing servers and clients to agree on the promisor remotes they can safely use.

The more servers use promisor remotes, the more it is important to properly control if they can lazy fetch when responding to a clone or fetch request from the client.

For example, in the context of large object promisors (see "Documentation/technical/large-object-promisors.adoc"), if a client clones with a filter set to 100kB while the server has moved all of the blobs >= 10kB to a promisor remote, the server will not be able to provide blobs between 10kB and 100kB to the client, which will make the clone fail.

Even if the `--filter=auto` option is available since ef2f1845ec (fetch-pack: wire up and enable auto filter logic, 2026-02-16) it's still a good idea to provide more control over lazy fetching on the server side to server operators, as lazy fetching on the server side could be useful in corporate environments.

Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), lazy fetching has been controlled by the `GIT_NO_LAZY_FETCH` environment variable. This is a boolean that is set to 'true' by default when calling `git upload-pack` for security reasons.

The main security issue on the server side is making sure the served repo itself is also trusted, as lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo. The operator of the server should decide and mark that trust, not the served repo itself, nor the client.

This series introduces a new 'uploadpack.lazyFetchTrusted' protected configuration variable similar to 'safe.directory' (see "Documentation/config/safe.adoc") to mark trusted repos where lazy fetching is allowed. As it is protected, this config variable will only take effect if it is set in global or system scope, so only server operators can control it.

Previous related work =====================

A previous series called "Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH" [1] took a different approach as it wanted to make it easier to allow lazy fetching from accepted promisor remotes. But after brian replied that he didn't think it was a good idea, and after thinking about this more, my opinion now is that some promisor remotes being accepted or not is not really relevant to the issue.

In my reply to brian, I said:

"""
Different features could be developed (in future work) to improve on
the current state:
    - a way for lazy fetching to work without reading config files,
triggering hooks, or doing potentially sensitive things,
    - an explicit way for operators to mark trusted repos (like
perhaps a server-side config the operator sets per-repo),
    - operator-defined allow/deny rules, or maybe
    - some ways/scripts/commands to scan repos and check configuration
information, remote settings and everything potentially sensitive to
decide if a repo looks safe enough to allow lazy fetching or not.
"""

So I decided to go with "an explicit way for operators to mark trusted repos" and this series is an implementation of that.

Note that the feature developed in this series applies to protocol v0/v1 as well as v2 while the previous one was only related to v2.

[1]: https://lore.kernel.org/git/CAP8UFD0_S9eg_w42tcNRnT9E2ntLr_eHLnzE4c2dSu67DzZoXg@mail.gmail.com/

Overview of the patches =======================

  - Patch 1/5 is the only patch saved from the "Introduce a
    'fromAccepted' option to GIT_NO_LAZY_FETCH" series. It's not
    necessary for the rest of this series and its main feature to
    work, but I think it's a nice refactoring related to lazy
    fetching, so it might as well be part of this series. There is a
    small change in the commit message (to not mention following
    commits) compared to the version in the previous series.

  - Patches 2/5 and 3/5 extract and modify code used by the
    'safe.directory' config variable in a path_allowlist_apply()
    function, so that this function can be reused to process
    'uploadpack.lazyFetchTrusted' in the next patch.

  - Patch 4/5 actually uses path_allowlist_apply() from a new
    upload_pack_lazy_fetch_trusted() function to process
    'uploadpack.lazyFetchTrusted', but the result from that processing
    isn't actually used to have a practical effect.

  - Patch 5/5 wires up the new upload_pack_lazy_fetch_trusted()
    function to decide if lazy fetching can actually be enabled.

CI tests ========

They all pass, see:

https://github.com/chriscool/git/actions/runs/31171494296

Range diff with previous series ===============================

The range diff with the previous ("Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH") series is not very interesting as only the first patch has been saved, but anyway here it is:

1:  8dd67ddaca ! 1:  b5b0836d19 promisor-remote: factor out lazy_fetch_objects()
    @@ Commit message
         that could not be fetched are promisor objects.
     
         Let's refactor the lazy fetching logic out of these two functions
    -    into a new lazy_fetch_objects() function. This will make it easier
    -    to extend the lazy fetching logic in following commits.
    +    into a new lazy_fetch_objects() function.
     
         This is a pure refactoring with no intended behavior change. Two
         things shift in ways that are observably equivalent though:
2:  314c61cbbe < -:  ---------- promisor-remote: introduce enum allow_lazy_fetch
3:  cb2f5447e2 < -:  ---------- promisor-remote: teach 'fromAccepted' to GIT_NO_LAZY_FETCH
-:  ---------- > 2:  879e3a34e3 setup: extract path_allowlist_apply()
-:  ---------- > 3:  98431ab7b3 setup: add 'allow_dot' arg to path_allowlist_apply()
-:  ---------- > 4:  a46f4c1bb8 upload-pack: read uploadpack.lazyFetchTrusted
-:  ---------- > 5:  4063f233aa builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo

Christian Couder (5):
  promisor-remote: factor out lazy_fetch_objects()
  setup: extract path_allowlist_apply()
  setup: add 'allow_dot' arg to path_allowlist_apply()
  upload-pack: read uploadpack.lazyFetchTrusted
  builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo

 Documentation/config/uploadpack.adoc  |  42 ++++++++++
 Documentation/git-upload-pack.adoc    |   5 ++
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  11 +++
 promisor-remote.c                     |  76 ++++++++++--------
 setup.c                               | 108 ++++++++++++++------------
 setup.h                               |  28 +++++++
 t/t5710-promisor-remote-capability.sh |  70 +++++++++++++++++
 upload-pack.c                         |  37 +++++++++
 upload-pack.h                         |   3 +
 10 files changed, 304 insertions(+), 80 deletions(-)

In "promisor-remote.c:fetch_objects()", there is a check to disable lazy fetching when the `GIT_NO_LAZY_FETCH` environment variable is set. The fetch_objects() function is called once per promisor remote though. So the check might be performed more times than necessary.

Also promisor_remote_get_direct() mixes up the logic deciding which promisor remotes to try with the logic checking that the objects that could not be fetched are promisor objects.

Let's refactor the lazy fetching logic out of these two functions into a new lazy_fetch_objects() function.

This is a pure refactoring with no intended behavior change. Two things shift in ways that are observably equivalent though:

  - the `GIT_NO_LAZY_FETCH` check is now performed once up front,
    instead of once per promisor remote, and

  - promisor_remote_init() is no longer called when lazy fetching
    is disabled, which is fine as nothing downstream of it, like
    is_promisor_object(), needs it in that case.

While at it, let's also convert try_promisor_remotes() to return 'bool' instead of 'int', as it just returns whether all the objects could be fetched, and document its return value.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 76 ++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

In a following commit we are going to check whether a repository is part of an allowlist specified in a config variable.

To prepare for that let's extract existing code from safe_directory_cb() into a new path_allowlist_apply() helper that will help with such checks.

While at it let's make the helper's code simpler and more generic.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 setup.c | 107 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 59 insertions(+), 48 deletions(-)

Previous commits created and prepared the path_allowlist_apply() function.

Let's reuse this function for a new "uploadpack.lazyFetchTrusted" configuration variable.

It allows us to:

  - read an allowlist from that config variable,
  - check if the current repo is in that list, and
  - return the result from a new upload_pack_lazy_fetch_trusted()
    function.

The new function will be used in a following commit.

Note that the new config variable should be read only from protected configuration files.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 upload-pack.c | 37 +++++++++++++++++++++++++++++++++++++
 upload-pack.h |  3 +++
 2 files changed, 40 insertions(+)

A previous commit added a new "uploadpack.lazyFetchTrusted" protected config variable that can contain an allowlist of repos, as well as functions to check if the current repo is in that list. But when the current repo is in that list, we currently do nothing.

Let's instead set `GIT_NO_LAZY_FETCH` to `0`, which allows `upload-pack` and its `pack-objects` child process to lazily fetch the objects they need to serve a client, for example when the filter used by the client and the one used by the server don't match.

This allows server operators to properly control lazy fetching. It is their responsibility, not the client's, to decide if the served repo is trusted, as the main security issue is that lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo.

As `GIT_NO_LAZY_FETCH` is passed down to child processes through the environment, this works for `pack-objects`, which performs the lazy fetch when serving a client, without any further plumbing.

Now that "uploadpack.lazyFetchTrusted" is actually doing something, let's document it and reference it from GIT_NO_LAZY_FETCH's docs.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/config/uploadpack.adoc  | 42 ++++++++++++++++
 Documentation/git-upload-pack.adoc    |  5 ++
 Documentation/git.adoc                |  4 +-
 builtin/upload-pack.c                 | 11 +++++
 t/t5710-promisor-remote-capability.sh | 70 +++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)

A previous commit created path_allowlist_apply() with the goal of later reusing that function. But when it will be reused in a following commit this function will need to reject non-absolute paths including those with a single dot that are currently accepted.

To prepare for reusing path_allowlist_apply(), let's add a `bool allow_dot` argument to it, and let's export this function.

While at it let's document it properly in "setup.h".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 setup.c |  9 +++++----
 setup.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

On Fri, Aug 7, 2026 at 3:55 PM Christian Couder <christian.couder@gmail.com> wrote:

[...]

Sorry I just realized that there is the wrong sign-off email address again. Will fix it in v2.

Christian Couder <christian.couder@gmail.com> writes:

What's missing is the information on the base. I tried applying these patches to 'v2.55.0' and the recent tips of 'master':

    2c78326f81 The 11th batch
    5b2471720c The 10th batch
    a97fcc37c2 The 9th batch
    13c7afec21 The 8th batch
    9a0c4701dc The 7th batch
    5d2e770923 The 6th batch
    48bbf81c29 The 5th batch
    41365c2a9b The 4th batch for Git 2.56
    d35c5399e3 The 3rd batch for Git 2.56
    55526a1826 The 2nd batch for Git 2.56

but the series did not apply to any of them.

It turns out the reason has nothing to do with your choice of base. It is because the series structure is not understood by 'b4'.

The cover letter I am responding to is a reply to another series, but the patches in this round are not marked as 'v2'. This seems to cause 'b4' to grab patches from both series and smash them together, resulting in an inapplicable mess. It seems you cannot have your cake and eat it, too 😠.

Next time, please do not thread the two topics together unless you are marking the newer iteration with a higher 'vN' number.

Thanks.

On Fri, Aug 7, 2026 at 8:31 PM Junio C Hamano <gitster@pobox.com> wrote:

I guess b4 should have, or grow, an option for that, because it's not uncommon that someone would post an alternative patch or patch series in reply to some patch(es).

Ok, I will not do that. I will start a separate thread. Now I hope it will work if I send a v2 in reply to the latest series.

Thanks.

Christian Couder <christian.couder@gmail.com> writes:

There is an option that tells it not to crawl up the parent article to find siblings, and it would have worked fine in this case, but then it would prevent us from noticing that a newer iteration exists.

But it should not be the norm.

Recently the "promisor-remote" capability was added to protocol v2, allowing servers and clients to agree on the promisor remotes they can safely use.

The more servers use promisor remotes, the more it is important to properly control if they can lazy fetch when responding to a clone or fetch request from the client.

For example, in the context of large object promisors (see "Documentation/technical/large-object-promisors.adoc"), if a client clones with a filter set to 100kB while the server has moved all of the blobs >= 10kB to a promisor remote, the server will not be able to provide blobs between 10kB and 100kB to the client, which will make the clone fail.

Even if the `--filter=auto` option is available since ef2f1845ec (fetch-pack: wire up and enable auto filter logic, 2026-02-16) it's still a good idea to provide more control over lazy fetching on the server side to server operators, as lazy fetching on the server side could be useful in corporate environments.

Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), lazy fetching has been controlled by the `GIT_NO_LAZY_FETCH` environment variable. This is a boolean that is set to 'true' by default when calling `git upload-pack` for security reasons.

The main security issue on the server side is making sure the served repo itself is also trusted, as lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo. The operator of the server should decide and mark that trust, not the served repo itself, nor the client.

This series introduces a new 'uploadpack.lazyFetchTrusted' protected configuration variable similar to 'safe.directory' (see "Documentation/config/safe.adoc") to mark trusted repos where lazy fetching is allowed. As it is protected, this config variable will only take effect if it is set in global or system scope, so only server operators can control it.

Previous related work =====================

A previous series called "Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH" [1] took a different approach as it wanted to make it easier to allow lazy fetching from accepted promisor remotes. But after brian replied that he didn't think it was a good idea, and after thinking about this more, my opinion now is that some promisor remotes being accepted or not is not really relevant to the issue.

In my reply to brian, I said:

"""
Different features could be developed (in future work) to improve on
the current state:
    - a way for lazy fetching to work without reading config files,
triggering hooks, or doing potentially sensitive things,
    - an explicit way for operators to mark trusted repos (like
perhaps a server-side config the operator sets per-repo),
    - operator-defined allow/deny rules, or maybe
    - some ways/scripts/commands to scan repos and check configuration
information, remote settings and everything potentially sensitive to
decide if a repo looks safe enough to allow lazy fetching or not.
"""

So I decided to go with "an explicit way for operators to mark trusted repos" and this series is an implementation of that.

Note that the feature developed in this series applies to protocol v0/v1 as well as v2 while the previous one was only related to v2.

[1]: https://lore.kernel.org/git/CAP8UFD0_S9eg_w42tcNRnT9E2ntLr_eHLnzE4c2dSu67DzZoXg@mail.gmail.com/

Overview of the patches =======================

  - Patch 1/5 is the only patch saved from the "Introduce a
    'fromAccepted' option to GIT_NO_LAZY_FETCH" series. It's not
    necessary for the rest of this series and its main feature to
    work, but I think it's a nice refactoring related to lazy
    fetching, so it might as well be part of this series. There is a
    small change in the commit message (to not mention following
    commits) compared to the version in the previous series.

  - Patches 2/5 and 3/5 extract and modify code used by the
    'safe.directory' config variable in a path_allowlist_apply()
    function, so that this function can be reused to process
    'uploadpack.lazyFetchTrusted' in the next patch.

  - Patch 4/5 actually uses path_allowlist_apply() from a new
    upload_pack_lazy_fetch_trusted() function to process
    'uploadpack.lazyFetchTrusted', but the result from that processing
    isn't actually used to have a practical effect.

  - Patch 5/5 wires up the new upload_pack_lazy_fetch_trusted()
    function to decide if lazy fetching can actually be enabled.

Changes since v1 ================

The only change is that the Signed-off-by email address has been fixed to "christian.couder@gmail.com", which is my primary address in ".mailmap" since 6375b40aea (mailmap: change primary address for Christian Couder, 2026-08-03).

This version is also sent as a separate 'v2' iteration in reply to v1, instead of being threaded onto the previous "Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH" series, and it now contains a 'base-commit' trailer, so that 'b4' and other tools can find the right base and the right patches.

CI tests ========

I didn't run them as only commit messages changed since v1.

Range diff with v1 ==================

1:  b5b0836d19 ! 1:  1605740203 promisor-remote: factor out lazy_fetch_objects()
    @@ Commit message
         'bool' instead of 'int', as it just returns whether all the objects
         could be fetched, and document its return value.
     
    -    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
      ## promisor-remote.c ##
     @@ promisor-remote.c: static int fetch_objects(struct repository *repo,
2:  879e3a34e3 ! 2:  5f226b6508 setup: extract path_allowlist_apply()
    @@ Commit message
     
         While at it let's make the helper's code simpler and more generic.
     
    -    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
      ## setup.c ##
     @@ setup.c: static int canonicalize_ceiling_entry(struct string_list_item *item,
3:  98431ab7b3 ! 3:  051aa11fc9 setup: add 'allow_dot' arg to path_allowlist_apply()
    @@ Commit message
     
         While at it let's document it properly in "setup.h".
     
    -    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
      ## setup.c ##
     @@ setup.c: static int canonicalize_ceiling_entry(struct string_list_item *item,
4:  a46f4c1bb8 ! 4:  045b5e647b upload-pack: read uploadpack.lazyFetchTrusted
    @@ Commit message
         Note that the new config variable should be read only from protected
         configuration files.
     
    -    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
      ## upload-pack.c ##
     @@
5:  4063f233aa ! 5:  c116661202 builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo
    @@ Commit message
         Now that "uploadpack.lazyFetchTrusted" is actually doing something,
         let's document it and reference it from GIT_NO_LAZY_FETCH's docs.
     
    -    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
      ## Documentation/config/uploadpack.adoc ##
     @@ Documentation/config/uploadpack.adoc: uploadpack.allowRefInWant::

Christian Couder (5):
  promisor-remote: factor out lazy_fetch_objects()
  setup: extract path_allowlist_apply()
  setup: add 'allow_dot' arg to path_allowlist_apply()
  upload-pack: read uploadpack.lazyFetchTrusted
  builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo

 Documentation/config/uploadpack.adoc  |  42 ++++++++++
 Documentation/git-upload-pack.adoc    |   5 ++
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  11 +++
 promisor-remote.c                     |  76 ++++++++++--------
 setup.c                               | 108 ++++++++++++++------------
 setup.h                               |  28 +++++++
 t/t5710-promisor-remote-capability.sh |  70 +++++++++++++++++
 upload-pack.c                         |  37 +++++++++
 upload-pack.h                         |   3 +
 10 files changed, 304 insertions(+), 80 deletions(-)

base-commit: 745601a9a94110d74769ab605ccd4f61339758d2

In "promisor-remote.c:fetch_objects()", there is a check to disable lazy fetching when the `GIT_NO_LAZY_FETCH` environment variable is set. The fetch_objects() function is called once per promisor remote though. So the check might be performed more times than necessary.

Also promisor_remote_get_direct() mixes up the logic deciding which promisor remotes to try with the logic checking that the objects that could not be fetched are promisor objects.

Let's refactor the lazy fetching logic out of these two functions into a new lazy_fetch_objects() function.

This is a pure refactoring with no intended behavior change. Two things shift in ways that are observably equivalent though:

  - the `GIT_NO_LAZY_FETCH` check is now performed once up front,
    instead of once per promisor remote, and

  - promisor_remote_init() is no longer called when lazy fetching
    is disabled, which is fine as nothing downstream of it, like
    is_promisor_object(), needs it in that case.

While at it, let's also convert try_promisor_remotes() to return 'bool' instead of 'int', as it just returns whether all the objects could be fetched, and document its return value.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 promisor-remote.c | 76 ++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

In a following commit we are going to check whether a repository is part of an allowlist specified in a config variable.

To prepare for that let's extract existing code from safe_directory_cb() into a new path_allowlist_apply() helper that will help with such checks.

While at it let's make the helper's code simpler and more generic.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 setup.c | 107 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 59 insertions(+), 48 deletions(-)

A previous commit created path_allowlist_apply() with the goal of later reusing that function. But when it will be reused in a following commit this function will need to reject non-absolute paths including those with a single dot that are currently accepted.

To prepare for reusing path_allowlist_apply(), let's add a `bool allow_dot` argument to it, and let's export this function.

While at it let's document it properly in "setup.h".

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 setup.c |  9 +++++----
 setup.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

Previous commits created and prepared the path_allowlist_apply() function.

Let's reuse this function for a new "uploadpack.lazyFetchTrusted" configuration variable.

It allows us to:

  - read an allowlist from that config variable,
  - check if the current repo is in that list, and
  - return the result from a new upload_pack_lazy_fetch_trusted()
    function.

The new function will be used in a following commit.

Note that the new config variable should be read only from protected configuration files.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 upload-pack.c | 37 +++++++++++++++++++++++++++++++++++++
 upload-pack.h |  3 +++
 2 files changed, 40 insertions(+)

A previous commit added a new "uploadpack.lazyFetchTrusted" protected config variable that can contain an allowlist of repos, as well as functions to check if the current repo is in that list. But when the current repo is in that list, we currently do nothing.

Let's instead set `GIT_NO_LAZY_FETCH` to `0`, which allows `upload-pack` and its `pack-objects` child process to lazily fetch the objects they need to serve a client, for example when the filter used by the client and the one used by the server don't match.

This allows server operators to properly control lazy fetching. It is their responsibility, not the client's, to decide if the served repo is trusted, as the main security issue is that lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo.

As `GIT_NO_LAZY_FETCH` is passed down to child processes through the environment, this works for `pack-objects`, which performs the lazy fetch when serving a client, without any further plumbing.

Now that "uploadpack.lazyFetchTrusted" is actually doing something, let's document it and reference it from GIT_NO_LAZY_FETCH's docs.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 Documentation/config/uploadpack.adoc  | 42 ++++++++++++++++
 Documentation/git-upload-pack.adoc    |  5 ++
 Documentation/git.adoc                |  4 +-
 builtin/upload-pack.c                 | 11 +++++
 t/t5710-promisor-remote-capability.sh | 70 +++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)

Christian Couder <christian.couder@gmail.com> writes:

I do not know what you did, but it seems that an attempt to futz with the thread structure i.e.,

    This version is also sent as a separate 'v2' iteration in reply to v1,
    instead of being threaded onto the previous "Introduce a
    'fromAccepted' option to GIT_NO_LAZY_FETCH" series, and it now
    contains a 'base-commit' trailer, so that 'b4' and other tools can
    find the right base and the right patches.

made the patches totally unusable.

This [v2 0/5] has

    Message-ID: <20260813154748.2378747-1-christian.couder@gmail.com>
    In-reply-to: <20260807135511.1818458-1-christian.couder@gmail.com>

which is *correct*. It is a reply to [0/5] of the original thread. However, [v2 1/5] says:

    Message-ID: <20260813154748.2378747-2-christian.couder@gmail.com>
    In-reply-to: <20260807135511.1818458-1-christian.couder@gmail.com>

which is quite wrong. [v2 1/5] should be a reply to the cover letter of the v2 iteration. [v2 2/5], [v2 3/5], ... should also be replies to the cover letter of the v2 iteration.

I have never seen either plain vanilla send-email or GitGitGadget produce misthreaded series like this one. Do you have some custom settings to send things out?

Here is _one_ way to create a thread of the right shape:

 1. Develop this 5-patch series.

 2. Run

    $ git format-patch -v2 --cover-letter -5

    to grab 0000-cover-letter.patch to 0005-builtin-upload-...patch
    files.  You may also want to pass --range-diff option.

 3. Edit 0000-cover-letter.patch to your satisfaction.  Do not futz
    with In-Reply-To or References or Message-Id yourself there;
    the tool will do this part better than manual editing.

 4. Run

    $ git send-email --no-chain-reply-to \
      --in-reply-to='<20260807135511.1818458-1-christian.couder@gmail.com>' \
      000[0-5]-*.patch

    This will make the initial message (which is the cover letter of
    this iteration) a reply to the named message (which is the cover
    letter of the v1 iteration), and then the remainder replies to
    the initial message, which is what we want to see.

There surely are other right ways to do so. As long as the end result would look like

    * vN (1 < N) cover letter is a reply to v1 cover letter
    * vN patch M (0 < M) is a reply to vN cover letter

things will flow more smoothly.

HTH.

On Thu, Aug 13, 2026 at 10:31 PM Junio C Hamano <gitster@pobox.com> wrote:

Yeah, sorry. I tried to pass `--in-reply-to` to `git format-patch` instead of `git send-email` but it looks like they don't behave the same. Maybe because I have no `format.thread` set, so `git format-patch` defaults to `--no-thread`, and in that case `--in-reply-to` applies to every mail, not just the cover letter. I should have checked more carefully and not tried to improve too many things at once in my setup.

No, I just use `git format-patch` and `git send-email` without special configuration.

Yeah, that's pretty much what I usually do. I don't use `--no-chain-reply-to` though, but I will.

Yeah I should have checked more carefully before sending. Thanks.

Christian Couder <christian.couder@gmail.com> writes:

Perhaps you are contaminating format-patch output with In-Reply-To: and other message-id related headers (perhaps using its options)? I don't, and I suspect your mentees probably do not, as their patches do not have this issue, either.

Thanks.

Christian Couder <christian.couder@gmail.com> writes:

Perhaps writing it this way would make it easier to tell what is going on. We try the preferred ones first, and then fall back to the other ones.

        return (try_promisor_remotes(..., true) ||
                try_promisor_remotes(..., false));

But more importantly, I wonder if keeping the list of missing object names in memory will later turn out to be problematic in real-life applications. Without knowing much about how the current code for bulk dehydrating promisor objects is structured, I expected an API that looks more like:

 - bulk_download_begin(): performs the early part of
   fetch_objects(), sets up connections to the promisor remote(s),
   and calls start_command() on the child process.

 - bulk_download_this(): after calling the _begin() function above,
   it runs around and collects missing objects that it needs to do
   its work.  For each such missing object it discovers, this
   function is called, which sends the object name down the
   '--stdin' file descriptor.

 - bulk_download_done(): tells the child process that we are done
   feeding object names.

but that is not what I am seeing. I guess the current arrangement cannot be avoided, because we are going to fetch from more than one promisor remote. Under such constraints, the way to deal with a massive number of missing objects will not be "streaming" like I imagined above, but needs to be done differently, like spooling to a file or something silly like that.

In any case, except that this avoids checking the environment variable multiple times, I can see that it is a no-op refactoring of the existing code.

Nice and cleanly done.

Thanks.

Christian Couder <christian.couder@gmail.com> writes:

The inversion of the polarity from the original here is a nice touch. We no longer have to look at deeply indented block to tell immediately that nothing will happen when the configuration variable is not set.

The name "is_match" somehow feels a bit awkward. How about calling it

    *matches = true/false;

instead?

Christian Couder <christian.couder@gmail.com> writes:

If this is just "I want to add an extra caller that has specific need and do not care about others in the future", this may be OK but as a public function, this is a bit disappointing API design. I expected, as a generally useful function, you would instead add a callback function to allow replacing the use of is_absoute_path() plus the warning there, i.e.

void path_allowlist_apply(const char *key, const char *value,
			  const char *target_path, bool *matches,
			  bool (*allow_path)(const char *path))
{
	...

	if (!allow_path(allowed))
		goto end;

Also to avoid limiting this to configuration callback, I might recommend to have it be more like this:

void path_allowlist_apply(const char *allowed, const char *target_path,
			  bool *matches,
			  bool (*allow_path)(const char *path, void *cbdata),
			  void *allow_path_cbdata)

where the original safe-directory thing may call git_config_pathname() to compute allowed before calling this helper, and pass the address of something like:

	struct { const char *key, *value } cbdata = {
		.key = key, .value = value;
	};

as the cbdata, and pass something like this

	static bool allow_safe_dir(const char *path, void *cbdata_)
	{
		struct { const char *key, *value } *cbdata = _cbdata;
		if (is_absoute_path(path) || !strcmp(path, ".")
			return true; /* ok */

		warning(_("%s '%s' not absolute"), cbdata->key, path);
		return false;
	}

as the allow_path callback function. IOW warning, or insisting on it being absolute, etc., does not have to be carved in stone.

Thanks.

Christian Couder <christian.couder@gmail.com> writes:

OK.

I am not sure if the idea of configuration variable is truly sound, but if it were, I agree that this is a reasonable implementation for it.

Thanks.

Christian Couder <christian.couder@gmail.com> writes:

To somebody who designed this mechanism, it may have been clear that you are talking about multi-valued configuration variable, i.e.,

	[uploadpack]
		lazyFetchTrusted = repo1
		lazyFetchTrusted = repo2
		...
		lazyFetchTrusted = repoN
		
but the "config entries specify repositories" can be misread to mean

	[uploadpack]
		lazyFetchTrusted = repo1 repo2 ... repoN

especially combined with the use of verb "list" in "Listing a repository here tells..." we see below.

	A multi-valued configuration variable, each of which names a
	repository that `upload-pack` is allowed to ...

or something, perhaps. Say that upfront to make sure readers won't waste their time wondering what the syntax is.

Also, how would one specify a repository? A URL? Remote nickname used in

	[remote "nick"] url = ...

configuration? Local directory that houses another repository? Something else?

It would be interesting to set it to point at itself. A client asks you to serve a pack, you find some objects you yourself do not have because you fetched lazily from the upstream, and you end up asking you if you have that object (U+1F61B Face with Stuck-Out Tongue 😛).

a -> an before `uploadpack.lazyFetchTrusted`.

Recently the "promisor-remote" capability was added to protocol v2, allowing servers and clients to agree on the promisor remotes they can safely use.

The more servers use promisor remotes, the more it is important to properly control if they can lazy fetch when responding to a clone or fetch request from the client.

For example, in the context of large object promisors (see "Documentation/technical/large-object-promisors.adoc"), if a client clones with a filter set to 100kB while the server has moved all of the blobs >= 10kB to a promisor remote, the server will not be able to provide blobs between 10kB and 100kB to the client, which will make the clone fail.

Even if the `--filter=auto` option is available since ef2f1845ec (fetch-pack: wire up and enable auto filter logic, 2026-02-16) it's still a good idea to provide more control over lazy fetching on the server side to server operators, as lazy fetching on the server side could be useful in corporate environments.

Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), lazy fetching has been controlled by the `GIT_NO_LAZY_FETCH` environment variable. This is a boolean that is set to 'true' by default when calling `git upload-pack` for security reasons.

The main security issue on the server side is making sure the served repo itself is also trusted, as lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo. The operator of the server should decide and mark that trust, not the served repo itself, nor the client.

This series introduces a new 'uploadpack.lazyFetchTrusted' protected configuration variable similar to 'safe.directory' (see "Documentation/config/safe.adoc") to mark trusted repos where lazy fetching is allowed. As it is protected, this config variable will only take effect if it is set in global or system scope, so only server operators can control it.

Previous related work =====================

A previous series called "Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH" [1] took a different approach as it wanted to make it easier to allow lazy fetching from accepted promisor remotes. But after brian replied that he didn't think it was a good idea, and after thinking about this more, my opinion now is that some promisor remotes being accepted or not is not really relevant to the issue.

In my reply to brian, I said:

"""
Different features could be developed (in future work) to improve on
the current state:
    - a way for lazy fetching to work without reading config files,
triggering hooks, or doing potentially sensitive things,
    - an explicit way for operators to mark trusted repos (like
perhaps a server-side config the operator sets per-repo),
    - operator-defined allow/deny rules, or maybe
    - some ways/scripts/commands to scan repos and check configuration
information, remote settings and everything potentially sensitive to
decide if a repo looks safe enough to allow lazy fetching or not.
"""

So I decided to go with "an explicit way for operators to mark trusted repos" and this series is an implementation of that.

Note that the feature developed in this series applies to protocol v0/v1 as well as v2 while the previous one was only related to v2.

[1]: https://lore.kernel.org/git/CAP8UFD0_S9eg_w42tcNRnT9E2ntLr_eHLnzE4c2dSu67DzZoXg@mail.gmail.com/

Overview of the patches =======================

  - Patch 1/5 is the only patch saved from the "Introduce a
    'fromAccepted' option to GIT_NO_LAZY_FETCH" series. It's not
    necessary for the rest of this series and its main feature to
    work, but I think it's a nice refactoring related to lazy
    fetching, so it might as well be part of this series. There is a
    small change in the commit message (to not mention following
    commits) compared to the version in the previous series.

  - Patch 2/5 extracts and modifies code used by the 'safe.directory'
    config variable in new path_allowlist_config_apply() and
    path_allowlist_apply() functions, so that these functions can be
    reused to process 'uploadpack.lazyFetchTrusted' in the next patch.

  - Patch 3/5 uses the new functions from the previous patch in a new
    upload_pack_lazy_fetch_trusted() function to process
    'uploadpack.lazyFetchTrusted', but the result from that processing
    isn't actually used to have a practical effect.

  - Patch 4/5, which is new in this v3, prevents infinite lazy fetch
    recursions that the following patch would otherwise make possible.
    If a repo is allowed to lazy fetch and one of its promisor remotes
    resolves back to it, for example if it is its own promisor remote
    as Junio noticed when reviewing v2, each nested `upload-pack`
    inherits `GIT_NO_LAZY_FETCH=0` and fetches again.

  - Patch 5/5 wires up the new upload_pack_lazy_fetch_trusted()
    function to decide if lazy fetching can actually be enabled.

Changes since v2 ================

Thanks to Junio for reviewing the previous version.

Rebased on top of 3cb9185f65 (The 22nd batch, 2026-09-02) as the previous version was based on a quite old commit: 745601a9a9 (mailmap: map Elijah Newren's current and previous work addresses, 2026-08-12) and I wanted to avoid possible merge issues.

 - Patch 1/5 has a small simplification in how try_promisor_remotes()
   is called first with its last argument set to 'true', and then with
   it set to 'false'. Both calls are now chained with `||`.

 - Patches 2/5 and 3/5 have been squashed together and reworked
   completely into the new patch 2/5, especially:

   - path_allowlist_apply() now has a
     `bool (*allow_path)(const char *path, void *cbdata)` argument so
     that callers can customize which paths they accept.

   - A new path_allowlist_config_apply() wrapper around
     path_allowlist_apply() has been added to avoid code duplication
     in the callers.

   - The `int *is_match` argument of path_allowlist_apply() has been
     changed to `bool *matches` and `int is_safe` in
     `struct safe_directory_data` has been changed to `bool safe`
     accordingly.

 - Patch 3/5 (previously 4/5) has a number of changes:

   - Its commit message has been improved and adapted to the 2 other
     big changes below.

   - It defines its own allow_trusted_path() function to customize the
     paths it accepts and pass that new function to the new functions
     from the previous commit that it uses.

   - The code and commit message have been changed so that a served
     repository is identified only by its git dir. We wrongly used to
     say that it could also be identified by its worktree, but
     `upload-pack` actually uses enter_repo(), so it doesn't know
     about worktrees.

   - `int trusted` has been changed to `bool trusted` and moved after
     the other field in `struct lazy_fetch_trusted`. This matches the
     changes to `bool *matches` and `bool safe` in the "setup.c" code.

 - Patch 4/5 is new and prevents infinite lazy fetch recursions, using
   a new `GIT_INTERNAL_LAZY_FETCH_DEPTH` environment variable to limit
   the nesting depth. See the patch 4/5 description above.

 - Patch 5/5 has a few changes:

   - The `uploadpack.lazyFetchTrusted` doc has been clarified, typo
     fixed, reorganized, and completed with information related to the
     changes in this v3, especially:
       - the fact that repos are identified by their git dir, and
       - that configuring a repo as its own remote is not a good idea.

   - Two tests have been added to make sure
     `uploadpack.lazyFetchTrusted` doesn't make infinite lazy fetch
     recursion possible, and to show that repos are identified by
     their git dir, and cannot be identified by a worktree.

CI tests ========

They all pass except for the "debian-11" one which keeps failing at the "install git in container" step with the following error:

```
E: Release file for http://deb.debian.org/debian-security/dists/bullseye-security/InRelease
is expired (invalid since 18h 34min 49s). Updates for this repository will not be applied.
Error: Process completed with exit code 100.
```

so it is very likely unrelated to this series.

See: https://github.com/chriscool/git/actions/runs/34232995230

Range-diff compared to v2 =========================

1:  1605740203 ! 1:  9403597855 promisor-remote: factor out lazy_fetch_objects()
    @@ promisor-remote.c: static int try_promisor_remotes(struct repository *repo,
     +	promisor_remote_init(repo);
     +
     +	/* Try accepted remotes first (those the server told us to use) */
    -+	if (try_promisor_remotes(repo, remaining_oids, remaining_nr,
    -+				 to_free, true))
    -+		return true;
    -+
     +	return try_promisor_remotes(repo, remaining_oids, remaining_nr,
    -+				    to_free, false);
    ++				    to_free, true) ||
    ++		try_promisor_remotes(repo, remaining_oids, remaining_nr,
    ++				     to_free, false);
      }
      
      void promisor_remote_get_direct(struct repository *repo,
2:  5f226b6508 < -:  ---------- setup: extract path_allowlist_apply()
3:  051aa11fc9 < -:  ---------- setup: add 'allow_dot' arg to path_allowlist_apply()
-:  ---------- > 2:  2155c4202d setup: extract path_allowlist_apply()
4:  045b5e647b ! 3:  37043ffeaf upload-pack: read uploadpack.lazyFetchTrusted
    @@ Commit message
         upload-pack: read uploadpack.lazyFetchTrusted
     
         Previous commits created and prepared the path_allowlist_apply()
    -    function.
    +    and path_allowlist_config_apply() functions, but used them only for the
    +    "safe.directory" configuration variable.
     
    -    Let's reuse this function for a new "uploadpack.lazyFetchTrusted"
    +    Let's reuse these functions for a new "uploadpack.lazyFetchTrusted"
         configuration variable.
     
         It allows us to:
    @@ Commit message
           - return the result from a new upload_pack_lazy_fetch_trusted()
             function.
     
    -    The new function will be used in a following commit.
    +    As path_allowlist_config_apply() lets each caller decide which paths
    +    it is willing to accept using a callback, let's pass it a new
    +    allow_trusted_path() callback. Unlike the "safe.directory" callback, it
    +    accepts only absolute paths, and not ".", as `upload-pack` always
    +    serves a repository given by an absolute path, so there is no "current
    +    repository" for "." to refer to.
    +
    +    Note that a served repository is identified by its git directory, and
    +    not by its worktree. This is because `upload-pack` uses enter_repo()
    +    instead of the usual repository discovery, so it never learns about a
    +    worktree and `r->worktree` is always NULL there. In practice this
    +    means that a non-bare repository served as "/srv/repo" has to be
    +    allowlisted as "/srv/repo/.git".
    +
    +    The new upload_pack_lazy_fetch_trusted() function will be used in a
    +    following commit.
     
         Note that the new config variable should be read only from protected
         configuration files.
    @@ upload-pack.c: static int upload_pack_config(const char *var, const char *value,
      	return parse_hide_refs_config(var, value, "uploadpack", &data->hidden_refs);
      }
      
    ++/*
    ++ * Only absolute paths make sense here. Unlike 'safe.directory', "."
    ++ * is not accepted, as the served repository is always identified by
    ++ * an absolute path.
    ++ */
    ++static bool allow_trusted_path(const char *path, void *cbdata_)
    ++{
    ++	struct path_allowlist_cb_data *cbdata = cbdata_;
    ++
    ++	if (is_absolute_path(path))
    ++		return true;
    ++
    ++	warning(_("%s '%s' not absolute"), cbdata->key, path);
    ++	return false;
    ++}
    ++
     +struct lazy_fetch_trusted {
    -+	int trusted;
     +	char *repo_path;
    ++	bool trusted;
     +};
     +
     +static int upload_pack_protected_lazy_fetch_config(const char *var, const char *value,
    @@ upload-pack.c: static int upload_pack_config(const char *var, const char *value,
     +						   void *cb_data)
     +{
     +	struct lazy_fetch_trusted *data = cb_data;
    ++	struct path_allowlist_cb_data cbdata = { .key = var };
     +
    -+	if (!strcmp("uploadpack.lazyfetchtrusted", var)) {
    -+		path_allowlist_apply(var, value, data->repo_path,
    -+				     &data->trusted, false);
    ++	if (strcmp("uploadpack.lazyfetchtrusted", var))
     +		return 0;
    -+	}
    ++
    ++	path_allowlist_config_apply(var, value, data->repo_path, &data->trusted,
    ++				    allow_trusted_path, &cbdata);
     +
     +	return 0;
     +}
    @@ upload-pack.c: static int upload_pack_config(const char *var, const char *value,
     +{
     +	struct lazy_fetch_trusted data = { 0 };
     +
    -+	data.repo_path = real_pathdup(r->worktree ? r->worktree : r->gitdir, 0);
    ++	/*
    ++	 * A served repository is identified by its git directory, as
    ++	 * `upload-pack` uses enter_repo() instead of the usual repository
    ++	 * discovery, so its worktree, if any, is never known here.
    ++	 */
    ++	data.repo_path = real_pathdup(r->gitdir, 0);
     +	if (!data.repo_path)
     +		return false;
     +
-:  ---------- > 4:  38fc060999 promisor-remote: prevent infinite recursion when lazy fetching
5:  c116661202 ! 5:  8cb97230e5 builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo
    @@ Documentation/config/uploadpack.adoc: uploadpack.allowRefInWant::
      	replication delay.
     +
     +uploadpack.lazyFetchTrusted::
    -+	These config entries specify repositories that `upload-pack` is
    -+	allowed to lazily fetch missing objects for. By default,
    -+	`upload-pack` refuses to lazily fetch (see the description of the
    -+	`GIT_NO_LAZY_FETCH` environment variable in
    -+	linkgit:git-upload-pack[1]), because doing so would run `git fetch`,
    -+	which may execute arbitrary commands specified in the configuration
    -+	and hooks of the served repository. Listing a repository here tells
    -+	`upload-pack` that it is trusted, so lazy fetching from the promisor
    -+	remotes configured in it is allowed. This is equivalent to setting
    -+	`GIT_NO_LAZY_FETCH` to `0` for the matching repositories. An
    -+	explicitly set `GIT_NO_LAZY_FETCH` takes precedence over this
    -+	setting.
    ++	A multi-valued configuration variable, each of which contains the
    ++	absolute local path of a repository that `upload-pack` is allowed to
    ++	lazily fetch missing objects for.
    +++
    ++A repository is identified by its git directory, i.e. the `.git`
    ++directory of a repository that has a worktree, or the repository itself
    ++if it is bare. So a non-bare repository served as `/srv/repo` has to be
    ++allowlisted as `/srv/repo/.git`. Giving a path with `/*` appended to it
    ++will trust all repositories under the named directory. To trust all
    ++served repositories, set `uploadpack.lazyFetchTrusted` to the string
    ++`*`.
    +++
    ++The value of this setting is interpolated, i.e. `~/<path>` expands to a
    ++path relative to the home directory and `%(prefix)/<path>` expands to a
    ++path relative to Git's (runtime) prefix.
    +++
    ++By default, `upload-pack` refuses to lazily fetch (see the description
    ++of the `GIT_NO_LAZY_FETCH` environment variable in
    ++linkgit:git-upload-pack[1]), because doing so would run `git fetch`,
    ++which may execute arbitrary commands specified in the configuration
    ++and hooks of the served repository. Listing a repository here tells
    ++`upload-pack` that it is trusted, so lazy fetching from the promisor
    ++remotes configured in it is allowed. This is equivalent to setting
    ++`GIT_NO_LAZY_FETCH` to `0` for the matching repositories. An
    ++explicitly set `GIT_NO_LAZY_FETCH` takes precedence over this setting.
     ++
     +Note that this allows lazy fetching from any promisor remote
     +configured in the served repository, not only from the promisor
    @@ Documentation/config/uploadpack.adoc: uploadpack.allowRefInWant::
     +is trusted as a whole, including its configuration, so the promisor
     +remotes it configures are trusted too. It is the server operator's
     +responsibility to make sure that the promisor remotes of a trusted
    -+repository are also trustworthy.
    ++repository are also trustworthy. In particular, a trusted repository
    ++should not be configured as its own promisor remote, as `upload-pack`
    ++would then try to lazily fetch missing objects from the repository
    ++itself, which is pointless.
     ++
    -+This is a multi-valued setting, i.e. you can add more than one
    ++As this is a multi-valued setting, you can add more than one
     +repository via `git config (--global|--system) --add`. To reset the
     +list of trusted repositories (e.g. to override any such repositories
    -+specified in the system config), add a `uploadpack.lazyFetchTrusted`
    ++specified in the system config), add an `uploadpack.lazyFetchTrusted`
     +entry with an empty value.
     ++
    -+A repository is identified by its worktree, or its git directory for a bare
    -+repository, and the value must be an absolute path. Giving a path with `/*`
    -+appended to it will trust all repositories under the named directory. To trust
    -+all served repositories, set `uploadpack.lazyFetchTrusted` to the string `*`.
    -++
    -+The value of this setting is interpolated, i.e. `~/<path>` expands to a
    -+path relative to the home directory and `%(prefix)/<path>` expands to a
    -+path relative to Git's (runtime) prefix.
    -++
    -+Note that this configuration variable is only respected when it is specified
    -+in protected configuration (see <<SCOPES>>). This prevents untrusted
    -+repositories from tampering with this value.
    ++Note that this configuration variable is only respected when it is
    ++specified in protected configuration (see <<SCOPES>>). This prevents
    ++untrusted repositories from tampering with this value.
     
      ## Documentation/git-upload-pack.adoc ##
     @@ Documentation/git-upload-pack.adoc: This is implemented by having `upload-pack` internally set the
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
     +	# Check that the largest object is still missing on the server
     +	check_missing_objects server 1 "$oid"
     +'
    ++
    ++test_expect_success "trusted repo as its own promisor remote does not recurse" '
    ++	# No promisors are advertised
    ++	git -C server config promisor.advertise false &&
    ++	test_when_finished "rm -rf client" &&
    ++
    ++	# Add itself as its own remote
    ++	git -C server remote add self "$TRASH_DIRECTORY_URL/server" &&
    ++	git -C server config remote.self.promisor true &&
    ++	test_when_finished "git -C server remote remove self" &&
    ++
    ++	# Make "self" the only promisor remote of the server, so that it
    ++	# cannot get the missing object from "lop". Note that
    ++	# "remote.lop.partialCloneFilter" also makes "lop" a promisor
    ++	# remote, so it has to be unset too.
    ++	git -C server config --unset remote.lop.promisor &&
    ++	test_when_finished "git -C server config remote.lop.promisor true" &&
    ++	lop_filter="$(git -C server config remote.lop.partialCloneFilter)" &&
    ++	git -C server config --unset remote.lop.partialCloneFilter &&
    ++	test_when_finished "git -C server config remote.lop.partialCloneFilter \"$lop_filter\"" &&
    ++
    ++	# Allow lazy fetching from itself
    ++	test_config_global uploadpack.lazyFetchTrusted "$(pwd)/server" &&
    ++
    ++	# Check that lazy fetching fails
    ++	test_must_fail git clone --no-local --filter="blob:limit=5k" server client 2>err &&
    ++	test_grep "too many nested lazy fetches" err &&
    ++
    ++	# Check that the largest object is still missing on the server
    ++	check_missing_objects server 1 "$oid"
    ++'
    ++
    ++test_expect_success "uploadpack.lazyFetchTrusted needs the git dir of a non-bare repo" '
    ++	test_when_finished "rm -rf nonbare client client2" &&
    ++
    ++	# Create a non-bare repo, without any worktree content, so that
    ++	# its largest object can be filtered out below
    ++	git init nonbare &&
    ++	git -C nonbare remote add origin "$TRASH_DIRECTORY_URL/template" &&
    ++	git -C nonbare fetch origin &&
    ++	git -C nonbare update-ref HEAD FETCH_HEAD &&
    ++
    ++	git -C nonbare remote add lop "$TRASH_DIRECTORY_URL/lop" &&
    ++	git -C nonbare config remote.lop.promisor true &&
    ++	git -C nonbare config uploadpack.allowFilter true &&
    ++	git -C nonbare config uploadpack.allowAnySHA1InWant true &&
    ++	git -C nonbare config promisor.advertise false &&
    ++
    ++	# Repack everything, then repack without the largest object and
    ++	# create a promisor pack, like initialize_server() does
    ++	git -C nonbare -c repack.writebitmaps=false repack -a -d &&
    ++	rm -f nonbare/.git/objects/pack/*.promisor &&
    ++	git -C nonbare -c repack.writebitmaps=false repack -a -d \
    ++		--filter=blob:limit=5k --filter-to="$(pwd)/nonbare-pack" &&
    ++	promisor_file=$(ls nonbare/.git/objects/pack/*.pack | sed "s/\.pack/.promisor/") &&
    ++	>"$promisor_file" &&
    ++	check_missing_objects nonbare 1 "$oid" &&
    ++
    ++	# The worktree path does not identify the repo, so it is not
    ++	# trusted and the clone fails
    ++	test_config_global uploadpack.lazyFetchTrusted "$(pwd)/nonbare" &&
    ++	test_must_fail git clone --no-local --filter="blob:limit=1k" \
    ++		nonbare client 2>err &&
    ++	test_grep "lazy fetching disabled" err &&
    ++	check_missing_objects nonbare 1 "$oid" &&
    ++
    ++	# The git dir identifies the repo, so it is trusted and the
    ++	# clone succeeds
    ++	test_config_global uploadpack.lazyFetchTrusted "$(pwd)/nonbare/.git" &&
    ++	git clone --no-local --filter="blob:limit=1k" nonbare client2 &&
    ++	check_missing_objects nonbare 0 ""
    ++'
     +
      test_expect_success "init + fetch with promisor.advertise set to 'true'" '
      	git -C server config promisor.advertise true &&

Christian Couder (5):
  promisor-remote: factor out lazy_fetch_objects()
  setup: extract path_allowlist_apply()
  upload-pack: read uploadpack.lazyFetchTrusted
  promisor-remote: prevent infinite recursion when lazy fetching
  builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo

 Documentation/config/uploadpack.adoc  |  49 +++++++++
 Documentation/git-upload-pack.adoc    |   5 +
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  11 ++
 environment.h                         |   8 ++
 promisor-remote.c                     |  96 +++++++++++------
 setup.c                               | 138 ++++++++++++++++---------
 setup.h                               |  50 +++++++++
 t/t0410-partial-clone.sh              |  33 ++++++
 t/t5710-promisor-remote-capability.sh | 142 ++++++++++++++++++++++++++
 upload-pack.c                         |  59 +++++++++++
 upload-pack.h                         |   3 +
 12 files changed, 514 insertions(+), 84 deletions(-)

base-commit: 3cb9185f65410273787f74333cc027d2ea5daada

In "promisor-remote.c:fetch_objects()", there is a check to disable lazy fetching when the `GIT_NO_LAZY_FETCH` environment variable is set. The fetch_objects() function is called once per promisor remote though. So the check might be performed more times than necessary.

Also promisor_remote_get_direct() mixes up the logic deciding which promisor remotes to try with the logic checking that the objects that could not be fetched are promisor objects.

Let's refactor the lazy fetching logic out of these two functions into a new lazy_fetch_objects() function.

This is a pure refactoring with no intended behavior change. Two things shift in ways that are observably equivalent though:

  - the `GIT_NO_LAZY_FETCH` check is now performed once up front,
    instead of once per promisor remote, and

  - promisor_remote_init() is no longer called when lazy fetching
    is disabled, which is fine as nothing downstream of it, like
    is_promisor_object(), needs it in that case.

While at it, let's also convert try_promisor_remotes() to return 'bool' instead of 'int', as it just returns whether all the objects could be fetched, and document its return value.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 promisor-remote.c | 74 +++++++++++++++++++++++++++--------------------
 1 file changed, 43 insertions(+), 31 deletions(-)

In a following commit we are going to check whether a repository is part of an allowlist specified in a config variable.

To prepare for that let's extract existing code from safe_directory_cb() into a new path_allowlist_apply() helper that will help with such checks.

While at it let's make the helper's code simpler and more generic, by passing it a `bool (*allow_path)(const char *path, void *cbdata)` function that decides if a path is acceptable by the caller.

To further simplify how to reuse that new helper, and avoid duplicating the config-value handling in a future commit, let's also introduce a path_allowlist_config_apply() helper.

For clarity, let's change the `int is_safe` to `bool safe` in `struct safe_directory_data`.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 setup.c | 138 ++++++++++++++++++++++++++++++++++++--------------------
 setup.h |  50 ++++++++++++++++++++
 2 files changed, 138 insertions(+), 50 deletions(-)

Previous commits created and prepared the path_allowlist_apply() and path_allowlist_config_apply() functions, but used them only for the "safe.directory" configuration variable.

Let's reuse these functions for a new "uploadpack.lazyFetchTrusted" configuration variable.

It allows us to:

  - read an allowlist from that config variable,
  - check if the current repo is in that list, and
  - return the result from a new upload_pack_lazy_fetch_trusted()
    function.

As path_allowlist_config_apply() lets each caller decide which paths it is willing to accept using a callback, let's pass it a new allow_trusted_path() callback. Unlike the "safe.directory" callback, it accepts only absolute paths, and not ".", as `upload-pack` always serves a repository given by an absolute path, so there is no "current repository" for "." to refer to.

Note that a served repository is identified by its git directory, and not by its worktree. This is because `upload-pack` uses enter_repo() instead of the usual repository discovery, so it never learns about a worktree and `r->worktree` is always NULL there. In practice this means that a non-bare repository served as "/srv/repo" has to be allowlisted as "/srv/repo/.git".

The new upload_pack_lazy_fetch_trusted() function will be used in a following commit.

Note that the new config variable should be read only from protected configuration files.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 upload-pack.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 upload-pack.h |  3 +++
 2 files changed, 62 insertions(+)

If a repository R is configured to lazy fetch from a promisor remote P which is also configured to in turn lazy fetch from R, there is an infinite recursion: R asks P for a missing object, P asks R for it, and so on. The simplest case of this is a repository configured as its own promisor remote.

This is not reachable when serving a repository by default, as `upload-pack` sets `GIT_NO_LAZY_FETCH` to 1, which makes the nested `upload-pack` refuse to lazily fetch. A following commit will let server operators allow lazy fetching for repositories they trust though, and as `GIT_NO_LAZY_FETCH` is then set to 0 and passed down to child processes, nothing stops the recursion anymore.

It does not recurse forever in practice, but only because each level adds one more variable to the environment of the child process, so after a while `exec()` fails with:

    fatal: cannot exec 'git-upload-pack ...': Argument list too long
    fatal: unable to fork

To avoid this pathological case altogether, let's use a new `GIT_INTERNAL_LAZY_FETCH_DEPTH` to count the recursion depth, and let's check that it doesn't exceed a MAX_LAZY_FETCH_DEPTH limit (set to 5 for now).

Note that some nesting is legitimate: when `git fetch` runs `index-pack`, it can lazily fetch REF_DELTA bases that are missing locally, so the limit should not be 1.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 environment.h            |  8 ++++++++
 promisor-remote.c        | 26 ++++++++++++++++++++++----
 t/t0410-partial-clone.sh | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 4 deletions(-)

A previous commit added a new "uploadpack.lazyFetchTrusted" protected config variable that can contain an allowlist of repos, as well as functions to check if the current repo is in that list. But when the current repo is in that list, we currently do nothing.

Let's instead set `GIT_NO_LAZY_FETCH` to `0`, which allows `upload-pack` and its `pack-objects` child process to lazily fetch the objects they need to serve a client, for example when the filter used by the client and the one used by the server don't match.

This allows server operators to properly control lazy fetching. It is their responsibility, not the client's, to decide if the served repo is trusted, as the main security issue is that lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo.

As `GIT_NO_LAZY_FETCH` is passed down to child processes through the environment, this works for `pack-objects`, which performs the lazy fetch when serving a client, without any further plumbing.

Now that "uploadpack.lazyFetchTrusted" is actually doing something, let's document it and reference it from GIT_NO_LAZY_FETCH's docs.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 Documentation/config/uploadpack.adoc  |  49 +++++++++
 Documentation/git-upload-pack.adoc    |   5 +
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  11 ++
 t/t5710-promisor-remote-capability.sh | 142 ++++++++++++++++++++++++++
 5 files changed, 210 insertions(+), 1 deletion(-)

On Fri, Aug 14, 2026 at 7:56 PM Junio C Hamano <gitster@pobox.com> wrote:

[...]

It was `int is_match` to match with `int is_safe` in `struct safe_directory_data`, as the function is called this way:

path_allowlist_apply(key, value, data->path, &data->is_safe);

But OK, I have changed both `int is_match` and `int is_safe` to `bool matches` and `bool safe` in the v3 I just sent.

Thanks.

On Fri, Aug 14, 2026 at 8:12 PM Junio C Hamano <gitster@pobox.com> wrote:

I thought that flags might be enough at least for some time, but I agree that it could soon make the code difficult to reason about, which is not a good thing for this kind of code.

I have tried to implement it like you suggest in the v3 I just sent.

Thanks.

On Fri, Aug 14, 2026 at 9:35 PM Junio C Hamano <gitster@pobox.com> wrote:

I have used that in the v3 I just sent.

The v3 has improved regarding this as I think it makes it clearer that repos are identified by having their git dir, or a parent directory of it, in this config variable.

Actually it happens that it could recursively lazy fetch in v2, but this has been fixed with a new patch and a few tests in v3. Thanks for the suggestion.

Fixed in v3.

Thanks.

On Fri, Aug 14, 2026 at 7:49 PM Junio C Hamano <gitster@pobox.com> wrote:

Yes, this is used in v3.

Yeah, I prefer to avoid working on a big refactoring in this area until we have evidence showing that there is a bottleneck here.

Thanks.

Christian Couder <christian.couder@gmail.com> writes:

Yeah, I too noticed these while reading the patch. The latter change may be a very good thing, in that the calling sequence around promisor_remote_init() seems to be anybody who needs to access the promisor remote information is expected to _init() the system beforehand. If it were "call _init() once at the very beginning and then do random things on promisor remotes", then moving its callsite may have to be done more carefully, but with the "user makes sure it is initialized beforehand" convention, the postimage of this patch follows the pattern exactly.

Meh.

The comment was not quite understandable, at least to me, especially around "from the (non-)accepted" part of the sentence.

Also "could be fetched" made it sound as if this were dry-run but isn't this function actually doing the fetching and reporting if everything got fetched or there are still objects remaining to be fetched?

    /*
     * fetch remaining objects (given in remaining_oids) from
     * the known promisor remotes.  If accepted_only is true,
     * ignore promisor remotes with .accepted member unset.
     * return true when all requested objects have been fetched,
     * false otherwise.
     */

The above only mentions half of how the remaining_oids parameter is used (i.e., only on the input side), but if we are adding a comment, we should document how remaining_oids and to_free are used as well.

The semantics of to_free in the entire callchain is especially tricky to describe correctly, I am afraid.

Christian Couder <christian.couder@gmail.com> writes:

I am not sure if this clarifies, though.

This is inherited from the preimage and not something you would want to fix in this patch, but I do not think ignoring missing path like this is healthy. You do not know if the path given is missing by design (i.e., the set of paths is union of paths that could exist) or if it is missing due to an error (i.e., a filesystem that should have been mounted is not mounted). In the latter case, ignoring it may make the system behave in a way that the user did not intend to.

Christian Couder <christian.couder@gmail.com> writes:

I hate to say this but I think is_safe was perfectly good. is_match was not quite grammatrical (it is either "matches" ir "is_a_match"), but "is_safe" is perfectly fine.

Christian Couder <christian.couder@gmail.com> writes:

Good.

Does it have to be "unsigned long", though? Just like oid_nr, I'd prefer to see a number whose range or signedness does not matter in practice be typed as platform natural "int". Even though one could argue that "anything_nr cannot be negative so it must be unsigned", or "int might be too small for some platforms" or "int or ulong have different width on different platforms", or even "anything we count we should count in size_t", I do not think any of them is a good argument against it, especially when the value we start with is 5 ;-).

Christian Couder <christian.couder@gmail.com> writes:

While I agree that it is a good idea to make it more lenient to work with remotes that are explicitly marked as trusted, it somehow feels a bit unnatural for a configuration variable, or a conclusion derived from the setting of a configuration variable, overriding an environment variable. Who is setting this environment variable in the first place?

If NO_LAZY_FETCH is what server operators set and export, I strongly suspect that not honoring it merely because the new variable could be used to give them a finer-grained control would be very surprising experience for them.

If the answer is "this never comes from the end-user or the server operator. We used to automatically set NO_LAZY_FETCH from the process that spawns uploadpack because we trusted nobody", then I'd imagine that we would prefer to see that code that automatically sets NO_LAZY_FETCH to inspect the configuration variable and to decide not to do so.

And I think that is what the code is doing (in other words, from a cursory read, I think the new code is doing the right thing and it is just the way how the above is explained that I found it iffy). We used to say "when serving a client, we do not lazy fetch what we are missing from our promisor remotes by setting NO_LAZY_FETCH" and it was unconditional.

I think what we want to happen is:

 * If the server operator has NO_LAZY_FETCH set, we honor it and do
   not do anything.

 * If the server operator does not have NO_LAZY_FETCH set, then we
   see if the configuration variable is there, and if there is, we
   let it take care of which promisor remote to allow by not futzing
   with NO_LAZY_FETCH ourselves.

 * Otherwise, we set and export NO_LAZY_FETCH just we used to.

and what you have in the patch is close enough to that (you left the historical "disable lazy fetch upfront" so worst case you export the thing twice which is not necessary).

If "the served repo" refers to where the client is fetching from, trusting that repository or not is up to the client; if they do not trust it, they should not be coming to you.

I may be misunderstanding what you are trying to say here, but what is up to the server operator to decide is if the promisor remotes, which the repo that is serving the client uses, is trustworthy, right?

I am not seeing what is in the postcontext of this hunk and in the precontext of the next hunk, but I wonder if we can just remove this xsetenv (without "no_lazy_fetch_set" variable at all) here ...

... and instead check the existing environment here, and do the choice from three possibilities I listed above here.

Other than that, this is a great endgame of the series.

Thanks.

On Tue, Sep 8, 2026 at 8:12 PM Junio C Hamano <gitster@pobox.com> wrote:

I agree that using a plain "int" seems like the most straightforward, but we don't have git_env_int() while we have git_env_ulong().

So would you be fine with something like:

    int depth = (int)git_env_ulong(LAZY_FETCH_DEPTH_ENVIRONMENT, 0);

which is similar to the following in builtin/pack-objects.c:

    name_hash_version = (int)git_env_ulong("GIT_TEST_NAME_HASH_VERSION", 1);

? Or do you think it's time to introduce git_env_int() in a preparatory patch?

Christian Couder <christian.couder@gmail.com> writes:

There are 13 existing callers, among which one that you found explicitly casts to int, but many others make assignments with implicit cast (e.g., members of bloom_settings used in commit-graph.c are of type uint32_t), and config.c reads GIT_TEST_INDEX_THREADS into an "int val" with implicit cast. progress.c:get_defalut_delay() does the same.

So I would say that it is up to you to pile on existing technical debt by mimicking config.c:repo_config_get_index_threads() and progress.c:get_default_delay(), or audit all callers of git_env_ulong() and migrate appropriate ones among them to use git_env_int(). From my cursory survey, I suspect that not many callers of git_get_ulong() would survive.

Thanks.

Recently the "promisor-remote" capability was added to protocol v2, allowing servers and clients to agree on the promisor remotes they can safely use.

The more servers use promisor remotes, the more it is important to properly control if they can lazy fetch when responding to a clone or fetch request from the client.

For example, in the context of large object promisors (see "Documentation/technical/large-object-promisors.adoc"), if a client clones with a filter set to 100kB while the server has moved all of the blobs >= 10kB to a promisor remote, the server will not be able to provide blobs between 10kB and 100kB to the client, which will make the clone fail.

Even if the `--filter=auto` option is available since ef2f1845ec (fetch-pack: wire up and enable auto filter logic, 2026-02-16) it's still a good idea to provide more control over lazy fetching on the server side to server operators, as lazy fetching on the server side could be useful in corporate environments.

Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), lazy fetching has been controlled by the `GIT_NO_LAZY_FETCH` environment variable. This is a boolean that is set to 'true' by default when calling `git upload-pack` for security reasons.

The main security issue on the server side is making sure the served repo itself is also trusted, as lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo. The operator of the server should decide and mark that trust, not the served repo itself, nor the client.

This series introduces a new 'uploadpack.lazyFetchTrusted' protected configuration variable similar to 'safe.directory' (see "Documentation/config/safe.adoc") to mark trusted repos where lazy fetching is allowed. As it is protected, this config variable will only take effect if it is set in global or system scope, so only server operators can control it.

Previous related work =====================

A previous series called "Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH" [1] took a different approach as it wanted to make it easier to allow lazy fetching from accepted promisor remotes. But after brian replied that he didn't think it was a good idea, and after thinking about this more, my opinion now is that some promisor remotes being accepted or not is not really relevant to the issue.

In my reply to brian, I said:

"""
Different features could be developed (in future work) to improve on
the current state:
    - a way for lazy fetching to work without reading config files,
triggering hooks, or doing potentially sensitive things,
    - an explicit way for operators to mark trusted repos (like
perhaps a server-side config the operator sets per-repo),
    - operator-defined allow/deny rules, or maybe
    - some ways/scripts/commands to scan repos and check configuration
information, remote settings and everything potentially sensitive to
decide if a repo looks safe enough to allow lazy fetching or not.
"""

So I decided to go with "an explicit way for operators to mark trusted repos" and this series is an implementation of that.

Note that the feature developed in this series applies to protocol v0/v1 as well as v2 while the previous one was only related to v2.

[1]: https://lore.kernel.org/git/CAP8UFD0_S9eg_w42tcNRnT9E2ntLr_eHLnzE4c2dSu67DzZoXg@mail.gmail.com/

Overview of the patches =======================

  - Patch 1/5 is the only patch saved from the "Introduce a
    'fromAccepted' option to GIT_NO_LAZY_FETCH" series. It's not
    necessary for the rest of this series and its main feature to
    work, but I think it's a nice refactoring related to lazy
    fetching, so it might as well be part of this series.

  - Patch 2/5 extracts and modifies code used by the 'safe.directory'
    config variable in new path_allowlist_config_apply() and
    path_allowlist_apply() functions, so that these functions can be
    reused to process 'uploadpack.lazyFetchTrusted' in the next patch.

  - Patch 3/5 uses the new functions from the previous patch in a new
    upload_pack_lazy_fetch_trusted() function to process
    'uploadpack.lazyFetchTrusted', but the result from that processing
    isn't actually used to have a practical effect.

  - Patch 4/5 prevents infinite lazy fetch recursions that the
    following patch would otherwise make possible. If a repo is
    allowed to lazy fetch and one of its promisor remotes resolves
    back to it, for example if it is its own promisor remote as Junio
    noticed when reviewing v2, each nested `upload-pack` inherits
    `GIT_NO_LAZY_FETCH=0` and fetches again.

  - Patch 5/5 wires up the new upload_pack_lazy_fetch_trusted()
    function to decide if lazy fetching can actually be enabled.

Changes since v3 ================

Thanks to Junio for reviewing previous versions of this series.

Rebased on top of 34f06850c1 (Merge tag 'l10n-2.56.0-v1' of https://github.com/git-l10n/git-po, 2026-09-27) as I wanted to avoid possible merge issues and be based on a stable commit close to v2.56.0.

Except for some functions and variables that are now typed using a regular `int` instead of a `bool` or an `unsigned long`, there are mostly commit message and code comment changes in the first 4 patches of this version compared to v3.

The most significant changes are in patch 5/5.

 - In patch 1/5:

   - The commit message explains why it's fine to not call
     promisor_remote_init() when lazy fetching is disabled.

   - The code comments documenting both try_promisor_remotes() and the
     new lazy_fetch_objects() function are improved.

   - These two functions now keep returning `int`, as before this
     series, instead of `bool` as in v3.

 - In patch 2/5, instead of changing `int is_safe` to `bool safe` in
   `struct safe_directory_data`, only the type of this member is
   changed from `int` to `bool`. The commit message also better
   explains this change.

 - Patch 3/5 is unchanged. 

 - In patch 4/5, the variable and function arguments called `depth`
   are changed from an `unsigned long` to a regular `int`.

 - In patch 5/5:

   - In the code, instead of checking if GIT_NO_LAZY_FETCH is already
     set at the beginning of the command, then unconditionally setting
     it to 1, and later setting it to 0 if the repo is trusted, we now
     only check if it is set after entering the repo, and set it to 1
     if it isn't and the repo isn't trusted.

   - The patch title is changed accordingly from "set
     GIT_NO_LAZY_FETCH to 0 on trusted repo" to "don't disable lazy
     fetching on trusted repo".

   - The commit message is changed too:

     - the patch is reframed as teaching the code that sets
       `GIT_NO_LAZY_FETCH` to consult the config, rather than config
       overriding the environment,

     - the 3 possible cases regarding `GIT_NO_LAZY_FETCH` are listed,

     - the "served repo is trusted" sentence is fixed to say that the
       operator vouches for the promisor remotes, configuration and
       hooks of the listed repo.

CI tests ========

They all pass, see:

https://github.com/chriscool/git/actions/runs/36414541860

Range-diff compared to v3 =========================

1:  9403597855 ! 1:  e7332d0aa4 promisor-remote: factor out lazy_fetch_objects()
    @@ Commit message
             instead of once per promisor remote, and
     
           - promisor_remote_init() is no longer called when lazy fetching
    -        is disabled, which is fine as nothing downstream of it, like
    -        is_promisor_object(), needs it in that case.
    +        is disabled.
     
    -    While at it, let's also convert try_promisor_remotes() to return
    -    'bool' instead of 'int', as it just returns whether all the objects
    -    could be fetched, and document its return value.
    +    The latter is fine because the convention around promisor_remote_init()
    +    is that whoever needs to access the promisor remote information is
    +    expected to initialize it beforehand, and not that it should be
    +    initialized once at the very beginning before doing random things on
    +    promisor remotes. So moving its call site into lazy_fetch_objects(),
    +    which is the only code that needs the promisor remotes here, follows
    +    that convention. Nothing downstream of it, like is_promisor_object(),
    +    needs it when lazy fetching is disabled.
    +
    +    While at it, let's document try_promisor_remotes() and the new
    +    lazy_fetch_objects() function, especially how their `remaining_oids`,
    +    `remaining_nr` and `to_free` arguments are used, as the ownership
    +    rules around `to_free` are easy to get wrong.
     
         Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
    @@ promisor-remote.c: static int remove_fetched_oids(struct repository *repo,
      	return remaining_nr;
      }
      
    --static int try_promisor_remotes(struct repository *repo,
    --				struct object_id **remaining_oids,
    --				int *remaining_nr, int *to_free,
    --				bool accepted_only)
     +/*
    -+ * Return 'true' if all the objects could be fetched from the
    -+ * (non-)accepted remotes, 'false' otherwise.
    ++ * Fetch the remaining objects (given in '*remaining_oids', which
    ++ * contains '*remaining_nr' object ids) from the known promisor
    ++ * remotes. If 'accepted_only' is true, ignore promisor remotes with
    ++ * their 'accepted' member unset.
    ++ *
    ++ * When a fetch from a remote fails, the objects that are still
    ++ * missing are computed, and '*remaining_oids' and '*remaining_nr' are
    ++ * updated accordingly before trying the next remote. In that case
    ++ * '*remaining_oids' points to a new array that this function
    ++ * allocated, and '*to_free' is set to 1 to tell the caller that it
    ++ * owns that array and should free it. '*to_free' should be 0 on the
    ++ * first call.
    ++ *
    ++ * Return 1 when all the requested objects have been fetched, 0
    ++ * otherwise.
     + */
    -+static bool try_promisor_remotes(struct repository *repo,
    -+				 struct object_id **remaining_oids,
    -+				 int *remaining_nr,
    -+				 int *to_free,
    -+				 bool accepted_only)
    + static int try_promisor_remotes(struct repository *repo,
    + 				struct object_id **remaining_oids,
    +-				int *remaining_nr, int *to_free,
    ++				int *remaining_nr,
    ++				int *to_free,
    + 				bool accepted_only)
      {
      	struct promisor_remote *r = repo->promisor_remote_config->promisors;
    - 
     @@ promisor-remote.c: static int try_promisor_remotes(struct repository *repo,
    - 				continue;
    - 			}
    - 		}
    --		return 1; /* all fetched */
    -+		return true; /* all fetched */
    - 	}
    --	return 0;
    -+	return false;
    -+}
    -+
    + 	return 0;
    + }
    + 
     +/*
    -+ * Return 'true' if all the objects could be fetched, 'false' otherwise.
    ++ * Lazily fetch the objects given in '*remaining_oids' from the
    ++ * promisor remotes, trying the accepted ones first. See
    ++ * try_promisor_remotes() above for how '*remaining_oids',
    ++ * '*remaining_nr' and '*to_free' are used.
    ++ *
    ++ * Return 1 when all the requested objects have been fetched, 0
    ++ * otherwise.
     + */
    -+static bool lazy_fetch_objects(struct repository *repo,
    -+			       struct object_id **remaining_oids,
    -+			       int *remaining_nr,
    -+			       int *to_free)
    ++static int lazy_fetch_objects(struct repository *repo,
    ++			      struct object_id **remaining_oids,
    ++			      int *remaining_nr,
    ++			      int *to_free)
     +{
     +	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0)) {
     +		static int warning_shown;
    @@ promisor-remote.c: static int try_promisor_remotes(struct repository *repo,
     +			warning_shown = 1;
     +			warning(_("lazy fetching disabled; some objects may not be available"));
     +		}
    -+		return false;
    ++		return 0;
     +	}
     +
     +	promisor_remote_init(repo);
    @@ promisor-remote.c: static int try_promisor_remotes(struct repository *repo,
     +				    to_free, true) ||
     +		try_promisor_remotes(repo, remaining_oids, remaining_nr,
     +				     to_free, false);
    - }
    - 
    ++}
    ++
      void promisor_remote_get_direct(struct repository *repo,
    + 				const struct object_id *oids,
    + 				int oid_nr)
     @@ promisor-remote.c: void promisor_remote_get_direct(struct repository *repo,
      	struct object_id *remaining_oids = (struct object_id *)oids;
      	int remaining_nr = oid_nr;
2:  2155c4202d ! 2:  b4a63e3e4e setup: extract path_allowlist_apply()
    @@ Commit message
         the config-value handling in a future commit, let's also introduce a
         path_allowlist_config_apply() helper.
     
    -    For clarity, let's change the `int is_safe` to `bool safe` in
    -    `struct safe_directory_data`.
    +    As the new path_allowlist_apply() function reports its result through
    +    a `bool *matches` argument, let's also change the `int is_safe` member
    +    of `struct safe_directory_data` to a `bool`, so that its address can
    +    be passed as that argument.
     
         Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
    @@ setup.c: static int canonicalize_ceiling_entry(struct string_list_item *item,
      struct safe_directory_data {
      	char *path;
     -	int is_safe;
    -+	bool safe;
    ++	bool is_safe;
      };
      
      static int safe_directory_cb(const char *key, const char *value,
    @@ setup.c: static int canonicalize_ceiling_entry(struct string_list_item *item,
     -			free(allowed);
     -		}
     -	}
    -+	path_allowlist_config_apply(key, value, data->path, &data->safe,
    ++	path_allowlist_config_apply(key, value, data->path, &data->is_safe,
     +				    allow_safe_dir, &cbdata);
      
      	return 0;
      }
    -@@ setup.c: static int ensure_valid_ownership(const char *gitfile,
    - 	git_protected_config(safe_directory_cb, &data);
    - 
    - 	free(data.path);
    --	return data.is_safe;
    -+	return data.safe;
    - }
    - 
    - void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
     
      ## setup.h ##
     @@ setup.h: struct startup_info {
3:  37043ffeaf = 3:  1e2d2d4b4f upload-pack: read uploadpack.lazyFetchTrusted
4:  38fc060999 ! 4:  3e88ec41a4 promisor-remote: prevent infinite recursion when lazy fetching
    @@ promisor-remote.c: struct promisor_remote_config {
      			 const char *remote_name,
      			 const struct object_id *oids,
     -			 int oid_nr)
    -+			 int oid_nr, unsigned long depth)
    ++			 int oid_nr, int depth)
      {
      	struct child_process child = CHILD_PROCESS_INIT;
      	int i;
    @@ promisor-remote.c: static int fetch_objects(struct repository *repo,
      		     "--filter=blob:none", "--stdin", NULL);
      	if (!repo_config_get_bool(repo, "promisor.quiet", &quiet) && quiet)
      		strvec_push(&child.args, "--quiet");
    -+	strvec_pushf(&child.env, "%s=%lu", LAZY_FETCH_DEPTH_ENVIRONMENT, depth + 1);
    ++	strvec_pushf(&child.env, "%s=%d", LAZY_FETCH_DEPTH_ENVIRONMENT, depth + 1);
      	if (start_command(&child))
      		die(_("promisor-remote: unable to fork off fetch subprocess"));
      	child_in = xfdopen(child.in, "w");
    -@@ promisor-remote.c: static bool try_promisor_remotes(struct repository *repo,
    - 				 struct object_id **remaining_oids,
    - 				 int *remaining_nr,
    - 				 int *to_free,
    -+				 unsigned long depth,
    - 				 bool accepted_only)
    +@@ promisor-remote.c: static int try_promisor_remotes(struct repository *repo,
    + 				struct object_id **remaining_oids,
    + 				int *remaining_nr,
    + 				int *to_free,
    ++				int depth,
    + 				bool accepted_only)
      {
      	struct promisor_remote *r = repo->promisor_remote_config->promisors;
    -@@ promisor-remote.c: static bool try_promisor_remotes(struct repository *repo,
    +@@ promisor-remote.c: static int try_promisor_remotes(struct repository *repo,
      	for (; r; r = r->next) {
      		if (accepted_only != r->accepted)
      			continue;
    @@ promisor-remote.c: static bool try_promisor_remotes(struct repository *repo,
      			if (*remaining_nr == 1)
      				continue;
      			*remaining_nr = remove_fetched_oids(repo, remaining_oids,
    -@@ promisor-remote.c: static bool try_promisor_remotes(struct repository *repo,
    - 	return false;
    +@@ promisor-remote.c: static int try_promisor_remotes(struct repository *repo,
    + 	return 0;
      }
      
     +#define MAX_LAZY_FETCH_DEPTH 5
     +
      /*
    -  * Return 'true' if all the objects could be fetched, 'false' otherwise.
    -  */
    -@@ promisor-remote.c: static bool lazy_fetch_objects(struct repository *repo,
    - 			       int *remaining_nr,
    - 			       int *to_free)
    +  * Lazily fetch the objects given in '*remaining_oids' from the
    +  * promisor remotes, trying the accepted ones first. See
    +@@ promisor-remote.c: static int lazy_fetch_objects(struct repository *repo,
    + 			      int *remaining_nr,
    + 			      int *to_free)
      {
    -+	unsigned long depth = git_env_ulong(LAZY_FETCH_DEPTH_ENVIRONMENT, 0);
    ++	int depth = (int)git_env_ulong(LAZY_FETCH_DEPTH_ENVIRONMENT, 0);
     +
      	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0)) {
      		static int warning_shown;
      		if (!warning_shown) {
    -@@ promisor-remote.c: static bool lazy_fetch_objects(struct repository *repo,
    - 		return false;
    +@@ promisor-remote.c: static int lazy_fetch_objects(struct repository *repo,
    + 		return 0;
      	}
      
     +	if (depth >= MAX_LAZY_FETCH_DEPTH) {
     +		static int warning_shown;
     +		if (!warning_shown) {
     +			warning_shown = 1;
    -+			warning(_("too many nested lazy fetches (%lu); "
    ++			warning(_("too many nested lazy fetches (%d); "
     +				  "is a promisor remote pointing at the repository itself?"),
     +				depth);
     +		}
    -+		return false;
    ++		return 0;
     +	}
     +
      	promisor_remote_init(repo);
5:  8cb97230e5 ! 5:  ad8814984b builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo
    @@ Metadata
     Author: Christian Couder <christian.couder@gmail.com>
     
      ## Commit message ##
    -    builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo
    +    builtin/upload-pack: don't disable lazy fetching on trusted repo
     
         A previous commit added a new "uploadpack.lazyFetchTrusted" protected
         config variable that can contain an allowlist of repos, as well as
         functions to check if the current repo is in that list. But when the
         current repo is in that list, we currently do nothing.
     
    -    Let's instead set `GIT_NO_LAZY_FETCH` to `0`, which allows
    -    `upload-pack` and its `pack-objects` child process to lazily fetch the
    -    objects they need to serve a client, for example when the filter used
    -    by the client and the one used by the server don't match.
    +    Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default,
    +    2024-04-16), `upload-pack` sets `GIT_NO_LAZY_FETCH` to 1 itself,
    +    unconditionally, because by default it shouldn't trust the repositories
    +    it serves. Lazily fetching runs `git fetch`, which may execute
    +    arbitrary commands specified in the configuration and hooks of the
    +    served repo.
     
    -    This allows server operators to properly control lazy fetching. It is
    -    their responsibility, not the client's, to decide if the served repo is
    -    trusted, as the main security issue is that lazily fetching runs `git
    -    fetch`, which may execute arbitrary commands specified in the
    -    configuration and hooks of the served repo.
    +    The new "uploadpack.lazyFetchTrusted" protected config variable is not
    +    about overriding an environment variable. It's rather about teaching
    +    the code that automatically sets `GIT_NO_LAZY_FETCH` (because it had no
    +    way to know if the served repo could be trusted) to look at the new
    +    config variable to find out if a server operator actually vouched for
    +    that repo.
    +
    +    Let's implement that, so we now have the following cases:
    +
    +      - if `GIT_NO_LAZY_FETCH` is already set, we honor it and leave it
    +        alone, as it comes from the server operator,
    +
    +      - otherwise, if the served repo is in the
    +        "uploadpack.lazyFetchTrusted" allowlist, we don't disable lazy
    +        fetching,
    +
    +      - otherwise, we disable lazy fetching, as we used to.
    +
    +    This allows `upload-pack` and its `pack-objects` child process to
    +    lazily fetch the objects they need to serve a client, for example when
    +    the filter used by the client and the one used by the server don't
    +    match.
    +
    +    Note that what a server operator vouches for by listing a repo there
    +    is that the promisor remotes this repo is configured to lazily fetch
    +    from, as well as its configuration and hooks, are trustworthy. Whether
    +    a client trusts the repo it fetches from is a separate matter, and up
    +    to the client.
     
         As `GIT_NO_LAZY_FETCH` is passed down to child processes through the
         environment, this works for `pack-objects`, which performs the lazy
    @@ Documentation/git.adoc: for full details.
     
      ## builtin/upload-pack.c ##
     @@ builtin/upload-pack.c: int cmd_upload_pack(int argc,
    - 		OPT_END()
    - 	};
    - 	unsigned enter_repo_flags = ENTER_REPO_ANY_OWNER_OK;
    -+	bool no_lazy_fetch_set;
    - 
      	packet_trace_identity("upload-pack");
      	disable_replace_refs();
      	save_commit_buffer = 0;
    -+
    -+	no_lazy_fetch_set = !!getenv(NO_LAZY_FETCH_ENVIRONMENT);
    - 	xsetenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 0);
    +-	xsetenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 0);
      
      	argc = parse_options(argc, argv, prefix, options, upload_pack_usage, 0);
    + 
     @@ builtin/upload-pack.c: int cmd_upload_pack(int argc,
      	if (!enter_repo(the_repository, dir, enter_repo_flags))
      		die("'%s' does not appear to be a git repository", dir);
      
     +	/*
    -+	 * Relax the GIT_NO_LAZY_FETCH=1 default if the served repo is in
    -+	 * the "uploadpack.lazyFetchTrusted" protected allowlist and
    -+	 * GIT_NO_LAZY_FETCH was not already set explicitly.
    ++	 * Lazily fetching while serving a client would run `git fetch`,
    ++	 * which may execute arbitrary commands from the configuration
    ++	 * and hooks of the served repo, so we disable it by default as
    ++	 * we trust nobody. There are two ways for a server operator to
    ++	 * allow it though:
    ++	 *
    ++	 *   - if GIT_NO_LAZY_FETCH is already set, we leave it alone and
    ++	 *     honor whatever the operator put there,
    ++	 *
    ++	 *   - otherwise, if the served repo is in the
    ++	 *     "uploadpack.lazyFetchTrusted" protected allowlist, we
    ++	 *     don't disable lazy fetching.
     +	 */
    -+	if (!no_lazy_fetch_set && upload_pack_lazy_fetch_trusted(the_repository))
    -+		xsetenv(NO_LAZY_FETCH_ENVIRONMENT, "0", 1);
    ++	if (!getenv(NO_LAZY_FETCH_ENVIRONMENT) &&
    ++	    !upload_pack_lazy_fetch_trusted(the_repository))
    ++		xsetenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
     +
      	switch (determine_protocol_version_server()) {
      	case protocol_v2:

Christian Couder (5):
  promisor-remote: factor out lazy_fetch_objects()
  setup: extract path_allowlist_apply()
  upload-pack: read uploadpack.lazyFetchTrusted
  promisor-remote: prevent infinite recursion when lazy fetching
  builtin/upload-pack: don't disable lazy fetching on trusted repo

 Documentation/config/uploadpack.adoc  |  49 +++++++++
 Documentation/git-upload-pack.adoc    |   5 +
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  19 +++-
 environment.h                         |   8 ++
 promisor-remote.c                     | 105 ++++++++++++++-----
 setup.c                               | 136 +++++++++++++++---------
 setup.h                               |  50 +++++++++
 t/t0410-partial-clone.sh              |  33 ++++++
 t/t5710-promisor-remote-capability.sh | 142 ++++++++++++++++++++++++++
 upload-pack.c                         |  59 +++++++++++
 upload-pack.h                         |   3 +
 12 files changed, 534 insertions(+), 79 deletions(-)

base-commit: 34f06850c16c7f7ac822b1adc71354f11b0f2ca3

In "promisor-remote.c:fetch_objects()", there is a check to disable lazy fetching when the `GIT_NO_LAZY_FETCH` environment variable is set. The fetch_objects() function is called once per promisor remote though. So the check might be performed more times than necessary.

Also promisor_remote_get_direct() mixes up the logic deciding which promisor remotes to try with the logic checking that the objects that could not be fetched are promisor objects.

Let's refactor the lazy fetching logic out of these two functions into a new lazy_fetch_objects() function.

This is a pure refactoring with no intended behavior change. Two things shift in ways that are observably equivalent though:

  - the `GIT_NO_LAZY_FETCH` check is now performed once up front,
    instead of once per promisor remote, and

  - promisor_remote_init() is no longer called when lazy fetching
    is disabled.

The latter is fine because the convention around promisor_remote_init() is that whoever needs to access the promisor remote information is expected to initialize it beforehand, and not that it should be initialized once at the very beginning before doing random things on promisor remotes. So moving its call site into lazy_fetch_objects(), which is the only code that needs the promisor remotes here, follows that convention. Nothing downstream of it, like is_promisor_object(), needs it when lazy fetching is disabled.

While at it, let's document try_promisor_remotes() and the new lazy_fetch_objects() function, especially how their `remaining_oids`, `remaining_nr` and `to_free` arguments are used, as the ownership rules around `to_free` are easy to get wrong.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 promisor-remote.c | 83 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 57 insertions(+), 26 deletions(-)

In a following commit we are going to check whether a repository is part of an allowlist specified in a config variable.

To prepare for that let's extract existing code from safe_directory_cb() into a new path_allowlist_apply() helper that will help with such checks.

While at it let's make the helper's code simpler and more generic, by passing it a `bool (*allow_path)(const char *path, void *cbdata)` function that decides if a path is acceptable by the caller.

To further simplify how to reuse that new helper, and avoid duplicating the config-value handling in a future commit, let's also introduce a path_allowlist_config_apply() helper.

As the new path_allowlist_apply() function reports its result through a `bool *matches` argument, let's also change the `int is_safe` member of `struct safe_directory_data` to a `bool`, so that its address can be passed as that argument.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 setup.c | 136 ++++++++++++++++++++++++++++++++++++--------------------
 setup.h |  50 +++++++++++++++++++++
 2 files changed, 137 insertions(+), 49 deletions(-)

Previous commits created and prepared the path_allowlist_apply() and path_allowlist_config_apply() functions, but used them only for the "safe.directory" configuration variable.

Let's reuse these functions for a new "uploadpack.lazyFetchTrusted" configuration variable.

It allows us to:

  - read an allowlist from that config variable,
  - check if the current repo is in that list, and
  - return the result from a new upload_pack_lazy_fetch_trusted()
    function.

As path_allowlist_config_apply() lets each caller decide which paths it is willing to accept using a callback, let's pass it a new allow_trusted_path() callback. Unlike the "safe.directory" callback, it accepts only absolute paths, and not ".", as `upload-pack` always serves a repository given by an absolute path, so there is no "current repository" for "." to refer to.

Note that a served repository is identified by its git directory, and not by its worktree. This is because `upload-pack` uses enter_repo() instead of the usual repository discovery, so it never learns about a worktree and `r->worktree` is always NULL there. In practice this means that a non-bare repository served as "/srv/repo" has to be allowlisted as "/srv/repo/.git".

The new upload_pack_lazy_fetch_trusted() function will be used in a following commit.

Note that the new config variable should be read only from protected configuration files.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 upload-pack.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 upload-pack.h |  3 +++
 2 files changed, 62 insertions(+)

If a repository R is configured to lazy fetch from a promisor remote P which is also configured to in turn lazy fetch from R, there is an infinite recursion: R asks P for a missing object, P asks R for it, and so on. The simplest case of this is a repository configured as its own promisor remote.

This is not reachable when serving a repository by default, as `upload-pack` sets `GIT_NO_LAZY_FETCH` to 1, which makes the nested `upload-pack` refuse to lazily fetch. A following commit will let server operators allow lazy fetching for repositories they trust though, and as `GIT_NO_LAZY_FETCH` is then set to 0 and passed down to child processes, nothing stops the recursion anymore.

It does not recurse forever in practice, but only because each level adds one more variable to the environment of the child process, so after a while `exec()` fails with:

    fatal: cannot exec 'git-upload-pack ...': Argument list too long
    fatal: unable to fork

To avoid this pathological case altogether, let's use a new `GIT_INTERNAL_LAZY_FETCH_DEPTH` to count the recursion depth, and let's check that it doesn't exceed a MAX_LAZY_FETCH_DEPTH limit (set to 5 for now).

Note that some nesting is legitimate: when `git fetch` runs `index-pack`, it can lazily fetch REF_DELTA bases that are missing locally, so the limit should not be 1.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 environment.h            |  8 ++++++++
 promisor-remote.c        | 26 ++++++++++++++++++++++----
 t/t0410-partial-clone.sh | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 4 deletions(-)

A previous commit added a new "uploadpack.lazyFetchTrusted" protected config variable that can contain an allowlist of repos, as well as functions to check if the current repo is in that list. But when the current repo is in that list, we currently do nothing.

Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), `upload-pack` sets `GIT_NO_LAZY_FETCH` to 1 itself, unconditionally, because by default it shouldn't trust the repositories it serves. Lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo.

The new "uploadpack.lazyFetchTrusted" protected config variable is not about overriding an environment variable. It's rather about teaching the code that automatically sets `GIT_NO_LAZY_FETCH` (because it had no way to know if the served repo could be trusted) to look at the new config variable to find out if a server operator actually vouched for that repo.

Let's implement that, so we now have the following cases:

  - if `GIT_NO_LAZY_FETCH` is already set, we honor it and leave it
    alone, as it comes from the server operator,

  - otherwise, if the served repo is in the
    "uploadpack.lazyFetchTrusted" allowlist, we don't disable lazy
    fetching,

  - otherwise, we disable lazy fetching, as we used to.

This allows `upload-pack` and its `pack-objects` child process to lazily fetch the objects they need to serve a client, for example when the filter used by the client and the one used by the server don't match.

Note that what a server operator vouches for by listing a repo there is that the promisor remotes this repo is configured to lazily fetch from, as well as its configuration and hooks, are trustworthy. Whether a client trusts the repo it fetches from is a separate matter, and up to the client.

As `GIT_NO_LAZY_FETCH` is passed down to child processes through the environment, this works for `pack-objects`, which performs the lazy fetch when serving a client, without any further plumbing.

Now that "uploadpack.lazyFetchTrusted" is actually doing something, let's document it and reference it from GIT_NO_LAZY_FETCH's docs.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 Documentation/config/uploadpack.adoc  |  49 +++++++++
 Documentation/git-upload-pack.adoc    |   5 +
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  19 +++-
 t/t5710-promisor-remote-capability.sh | 142 ++++++++++++++++++++++++++
 5 files changed, 217 insertions(+), 2 deletions(-)

On Tue, Sep 8, 2026 at 7:40 PM Junio C Hamano <gitster@pobox.com> wrote:

Yeah, I have tried to explain this in the commit message of the v4 I just sent.

try_promisor_remotes() is not converted to return 'bool' in v4 then.

The comment before try_promisor_remotes() is now the following in v4:

+/*
+ * Fetch the remaining objects (given in '*remaining_oids', which
+ * contains '*remaining_nr' object ids) from the known promisor
+ * remotes. If 'accepted_only' is true, ignore promisor remotes with
+ * their 'accepted' member unset.
+ *
+ * When a fetch from a remote fails, the objects that are still
+ * missing are computed, and '*remaining_oids' and '*remaining_nr' are
+ * updated accordingly before trying the next remote. In that case
+ * '*remaining_oids' points to a new array that this function
+ * allocated, and '*to_free' is set to 1 to tell the caller that it
+ * owns that array and should free it. '*to_free' should be 0 on the
+ * first call.
+ *
+ * Return 1 when all the requested objects have been fetched, 0
+ * otherwise.
+ */

I hope it's better.

Thanks.

On Tue, Sep 8, 2026 at 7:48 PM Junio C Hamano <gitster@pobox.com> wrote:

The change is now explained in the following way:

    +    As the new path_allowlist_apply() function reports its result through
    +    a `bool *matches` argument, let's also change the `int is_safe` member
    +    of `struct safe_directory_data` to a `bool`, so that its address can
    +    be passed as that argument.

and only the type of the variable is changed in v4. The "is_safe" original name is kept.

In dc0edbb01c (safe.directory: normalize the configured path, 2024-07-30) you say:

     - A configured safe.directory may be coming from .gitignore in the
       home directory that may be shared across machines.  The path
       meant to match with an entry may not necessarily exist on all of
       such machines, so not being able to convert them to real path on
       this machine is *not* a condition that is worthy of warning.
       Hence, we ignore a path that cannot be converted to a real path.

So I don't know what is the right thing to do. Maybe it's safer to warn by default but have a config option to not warn? Or maybe we should remember the unresolvable entries, and mention them only when the overall check fails?

Anyway I can add a NEEDSWORK here for now in a separate patch in this series or maybe in a followup series. In v4 nothing was changed regarding this.

On Wed, Sep 9, 2026 at 11:39 PM Junio C Hamano <gitster@pobox.com> wrote:

Let me pile on existing technical debt and explicitly cast to int with the following in v4 then:

 int depth = (int)git_env_ulong(LAZY_FETCH_DEPTH_ENVIRONMENT, 0);

Thanks.

On Tue, Sep 8, 2026 at 8:34 PM Junio C Hamano <gitster@pobox.com> wrote:

I have tried to improve on that in v4 by rewording the title and commit message.

This should be fixed in v4, see below.

I think that by listing a repo in uploadpack.lazyFetchTrusted, the operator vouches for the following:

- the repo's configuration and hooks, because git fetch will execute them,
- the promisor remotes it is configured to lazily fetch from, because
objects will come from there.

I have tried to clarify this in v4 with the following in the commit message:

    +    Note that what a server operator vouches for by listing a repo there
    +    is that the promisor remotes this repo is configured to lazily fetch
    +    from, as well as its configuration and hooks, are trustworthy. Whether
    +    a client trusts the repo it fetches from is a separate matter, and up
    +    to the client.

Yes, that's what is implemented in v4. The three possibilities are also listed in the commit message now.

Thanks.

Christian Couder <christian.couder@gmail.com> writes:

This might be a minor point (as not many people may be using the safe.directory feature that this was moved from), and this dates back two years, starting with dc0edbb01c (safe.directory: normalize the configured path, 2024-07-30), but the above design decision cuts both ways. If you misspelled a pathname, you would never be told about it.

I wonder if we want to allow users to explicitly mark that it is OK if a path does not exist, in much the same way that a pathname-typed configuration variable can be prefixed with :(optional) to tell the system "if this path exists on the system, use it, but if not, instead of warning, pretend that you did not see this specified".

That way, a user can first specify the value normally, and then when they reuse the .gitconfig file somewhere else that does not have the path, they see a warning message. You would help them by giving a hint, e.g.,

    Specified path foo/bar does not exist.  If you spelled the
    pathname correctly, and the path is allowed to be missing,
    mark it as optional, i.e., ":(optional)foo/bar".

or something along those lines in the warning message and the world would be a much better place.

In any case, it is outside the scope of this series, beyond leaving a NEEDSWORK comment here, and/or a #leftoverbits comment in the review.

Christian Couder <christian.couder@gmail.com> writes:

The diffstat above is pleasing to see, with ample documentation to help users, tests with (hopefully) reasonable coverage, and a minimal amount of actual code changes to enable the feature, thanks to the preparatory work done in earlier steps.

"each of which" lacks a plural noun to modify.  Perhaps

	each value of which specifies the absolute local path of a
	repository from which upload-pack is allowed to lazily fetch
	missing objects.

"i.e." -> "i.e.," (similarly "e.g." -> "e.g.," below).

OK, the logic is so trivially obvious and clear that it wouldn't even need the above comment. Very nice.

Recently the "promisor-remote" capability was added to protocol v2, allowing servers and clients to agree on the promisor remotes they can safely use.

The more servers use promisor remotes, the more it is important to properly control if they can lazy fetch when responding to a clone or fetch request from the client.

For example, in the context of large object promisors (see "Documentation/technical/large-object-promisors.adoc"), if a client clones with a filter set to 100kB while the server has moved all of the blobs >= 10kB to a promisor remote, the server will not be able to provide blobs between 10kB and 100kB to the client, which will make the clone fail.

Even if the `--filter=auto` option is available since ef2f1845ec (fetch-pack: wire up and enable auto filter logic, 2026-02-16) it's still a good idea to provide more control over lazy fetching on the server side to server operators, as lazy fetching on the server side could be useful in corporate environments.

Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), lazy fetching has been controlled by the `GIT_NO_LAZY_FETCH` environment variable. This is a boolean that is set to 'true' by default when calling `git upload-pack` for security reasons.

The main security issue on the server side is making sure the served repo itself is also trusted, as lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo. The operator of the server should decide and mark that trust, not the served repo itself, nor the client.

This series introduces a new 'uploadpack.lazyFetchTrusted' protected configuration variable similar to 'safe.directory' (see "Documentation/config/safe.adoc") to mark trusted repos where lazy fetching is allowed. As it is protected, this config variable will only take effect if it is set in global or system scope, so only server operators can control it.

Previous related work =====================

A previous series called "Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH" [1] took a different approach as it wanted to make it easier to allow lazy fetching from accepted promisor remotes. But after brian replied that he didn't think it was a good idea, and after thinking about this more, my opinion now is that some promisor remotes being accepted or not is not really relevant to the issue.

In my reply to brian, I said:

"""
Different features could be developed (in future work) to improve on
the current state:
    - a way for lazy fetching to work without reading config files,
triggering hooks, or doing potentially sensitive things,
    - an explicit way for operators to mark trusted repos (like
perhaps a server-side config the operator sets per-repo),
    - operator-defined allow/deny rules, or maybe
    - some ways/scripts/commands to scan repos and check configuration
information, remote settings and everything potentially sensitive to
decide if a repo looks safe enough to allow lazy fetching or not.
"""

So I decided to go with "an explicit way for operators to mark trusted repos" and this series is an implementation of that.

Note that the feature developed in this series applies to protocol v0/v1 as well as v2 while the previous one was only related to v2.

[1]: https://lore.kernel.org/git/CAP8UFD0_S9eg_w42tcNRnT9E2ntLr_eHLnzE4c2dSu67DzZoXg@mail.gmail.com/

Overview of the patches =======================

  - Patch 1/5 is the only patch saved from the "Introduce a
    'fromAccepted' option to GIT_NO_LAZY_FETCH" series. It's not
    necessary for the rest of this series and its main feature to
    work, but I think it's a nice refactoring related to lazy
    fetching, so it might as well be part of this series.

  - Patch 2/5 extracts and modifies code used by the 'safe.directory'
    config variable in new path_allowlist_config_apply() and
    path_allowlist_apply() functions, so that these functions can be
    reused to process 'uploadpack.lazyFetchTrusted' in the next patch.

  - Patch 3/5 uses the new functions from the previous patch in a new
    upload_pack_lazy_fetch_trusted() function to process
    'uploadpack.lazyFetchTrusted', but the result from that processing
    isn't actually used to have a practical effect.

  - Patch 4/5 prevents infinite lazy fetch recursions that the
    following patch would otherwise make possible. If a repo is
    allowed to lazy fetch and one of its promisor remotes resolves
    back to it, for example if it is its own promisor remote as Junio
    noticed when reviewing v2, each nested `upload-pack` inherits
    `GIT_NO_LAZY_FETCH=0` and fetches again.

  - Patch 5/5 wires up the new upload_pack_lazy_fetch_trusted()
    function to decide if lazy fetching can actually be enabled.

Changes since v4 ================

Thanks to Junio for reviewing previous versions of this series.

Rebased on top of a018953688 (Git 2.56, 2026-09-27) to be on a stable base.

There are no functional code changes compared to v4. Only code comments, documentation, tests and commit messages have changed, and those changes are relatively small.

 - In patch 2/5, a NEEDSWORK code comment has been added to say that
   we may want to warn in case of a missing path unless that path is
   marked with an ":(optional)" prefix. Also the commit message
   now mentions that NEEDSWORK code comment.

 - In the commit message of patch 3/5 and the documentation in patch
   5/5, the way a repository is identified by its git directory is
   worded more correctly and explained in more detail respectively. A
   test for the case where a repo is initialized using
   `--separate-git-dir=...` is added to the tests in patch 5/5.

 - In patch 5/5:

   - In both "uploadpack.adoc" and "git-upload-pack.adoc", as well as
     the commit message, it is now explained that while setting
     `GIT_NO_LAZY_FETCH` to 0 propagates to promisor remotes (on the
     same machine) which could also want to lazily fetch in turn,
     setting "uploadpack.lazyFetchTrusted" doesn't. A test is also
     added to check that.

   - "uploadpack.lazyFetchTrusted" is now described as "A multi-valued
     configuration variable, each value of which specifies the
     absolute local path of a repository ..." which fixes the grammar
     of "each of which".

   - "i.e." and "e.g." are now followed by a comma.

   - The test called "uploadpack.lazyFetchTrusted needs the git dir of
     a non-bare repo" now cleans up the "nonbare-pack-*" files it
     generates.

CI tests ========

They all pass, see:

https://github.com/chriscool/git/actions/runs/36861893004

Range-diff compared to v4 =========================

1:  e7332d0aa4 = 1:  d03dbba92c promisor-remote: factor out lazy_fetch_objects()
2:  b4a63e3e4e ! 2:  a067c2396b setup: extract path_allowlist_apply()
    @@ Commit message
     
         While at it let's make the helper's code simpler and more generic, by
         passing it a `bool (*allow_path)(const char *path, void *cbdata)`
    -    function that decides if a path is acceptable by the caller.
    +    function that decides if a path is acceptable by the caller, and let's
    +    add a NEEDSWORK comment about it silently ignoring missing, possibly
    +    misspelled, paths.
     
         To further simplify how to reuse that new helper, and avoid duplicating
         the config-value handling in a future commit, let's also introduce a
    @@ setup.c: static int canonicalize_ceiling_entry(struct string_list_item *item,
     +	 * exist as paths on all of these machines.  In other words,
     +	 * it is not a warning worthy event when there is no such path
     +	 * on this machine---the entry may be useful elsewhere.
    ++	 *
    ++	 * NEEDSWORK: this also silently ignores misspelled paths. We
    ++	 * may want to warn about a missing path unless it is marked
    ++	 * as allowed to be missing, e.g., with an ":(optional)"
    ++	 * prefix like pathname-typed configuration values, and hint
    ++	 * about that prefix in the warning.
     +	 */
     +	normalized = real_pathdup(allowed, 0);
     +	if (!normalized)
3:  1e2d2d4b4f ! 3:  1c489de88f upload-pack: read uploadpack.lazyFetchTrusted
    @@ Commit message
         instead of the usual repository discovery, so it never learns about a
         worktree and `r->worktree` is always NULL there. In practice this
         means that a non-bare repository served as "/srv/repo" has to be
    -    allowlisted as "/srv/repo/.git".
    +    allowlisted as "/srv/repo/.git" (or as the directory its ".git" file
    +    points to, if it has a ".git" file instead of a ".git" directory).
     
         The new upload_pack_lazy_fetch_trusted() function will be used in a
         following commit.
4:  3e88ec41a4 = 4:  69592b1b86 promisor-remote: prevent infinite recursion when lazy fetching
5:  ad8814984b ! 5:  6d0e72c357 builtin/upload-pack: don't disable lazy fetching on trusted repo
    @@ Commit message
         a client trusts the repo it fetches from is a separate matter, and up
         to the client.
     
    -    As `GIT_NO_LAZY_FETCH` is passed down to child processes through the
    -    environment, this works for `pack-objects`, which performs the lazy
    -    fetch when serving a client, without any further plumbing.
    +    As `pack-objects`, which performs the lazy fetch when serving a
    +    client, is a child process of `upload-pack`, not setting
    +    `GIT_NO_LAZY_FETCH` in `upload-pack` is enough for it to be allowed to
    +    lazily fetch, without any further plumbing.
    +
    +    On the other hand, as we leave `GIT_NO_LAZY_FETCH` unset for a trusted
    +    repo instead of setting it to 0, the trust doesn't propagate: if a
    +    trusted repo lazily fetches from a promisor remote that is itself
    +    served by `upload-pack` on the same machine, that nested `upload-pack`
    +    decides for its own repo. This is unlike when a server operator sets
    +    `GIT_NO_LAZY_FETCH` to 0, as that is inherited by all child processes.
     
         Now that "uploadpack.lazyFetchTrusted" is actually doing something,
    -    let's document it and reference it from GIT_NO_LAZY_FETCH's docs.
    +    let's document it (including this difference with `GIT_NO_LAZY_FETCH`),
    +    let's reference it from `GIT_NO_LAZY_FETCH`'s docs, and let's add tests
    +    for it.
     
         Signed-off-by: Christian Couder <christian.couder@gmail.com>
     
    @@ Documentation/config/uploadpack.adoc: uploadpack.allowRefInWant::
      	replication delay.
     +
     +uploadpack.lazyFetchTrusted::
    -+	A multi-valued configuration variable, each of which contains the
    -+	absolute local path of a repository that `upload-pack` is allowed to
    -+	lazily fetch missing objects for.
    ++	A multi-valued configuration variable, each value of which
    ++	specifies the absolute local path of a repository that
    ++	`upload-pack` is allowed to lazily fetch missing objects for.
     ++
    -+A repository is identified by its git directory, i.e. the `.git`
    -+directory of a repository that has a worktree, or the repository itself
    -+if it is bare. So a non-bare repository served as `/srv/repo` has to be
    -+allowlisted as `/srv/repo/.git`. Giving a path with `/*` appended to it
    -+will trust all repositories under the named directory. To trust all
    -+served repositories, set `uploadpack.lazyFetchTrusted` to the string
    -+`*`.
    ++A repository is identified by its git directory, after following any
    ++`.git` file and resolving symbolic links. That is the repository
    ++itself if it is bare, the `.git` directory of a repository that has a
    ++worktree, or the directory that a `.git` file points to, for example
    ++when the repository was created with `--separate-git-dir` or for a
    ++linked worktree (see linkgit:git-worktree[1]). So a non-bare
    ++repository served as `/srv/repo` usually has to be allowlisted as
    ++`/srv/repo/.git`. Giving a path with `/*` appended to it will trust
    ++all repositories under the named directory. To trust all served
    ++repositories, set `uploadpack.lazyFetchTrusted` to the string `*`.
     ++
    -+The value of this setting is interpolated, i.e. `~/<path>` expands to a
    -+path relative to the home directory and `%(prefix)/<path>` expands to a
    -+path relative to Git's (runtime) prefix.
    ++The value of this setting is interpolated, i.e., `~/<path>` expands to
    ++a path relative to the home directory and `%(prefix)/<path>` expands
    ++to a path relative to Git's (runtime) prefix.
     ++
     +By default, `upload-pack` refuses to lazily fetch (see the description
     +of the `GIT_NO_LAZY_FETCH` environment variable in
    @@ Documentation/config/uploadpack.adoc: uploadpack.allowRefInWant::
     +which may execute arbitrary commands specified in the configuration
     +and hooks of the served repository. Listing a repository here tells
     +`upload-pack` that it is trusted, so lazy fetching from the promisor
    -+remotes configured in it is allowed. This is equivalent to setting
    -+`GIT_NO_LAZY_FETCH` to `0` for the matching repositories. An
    ++remotes configured in it is allowed. This is similar to setting
    ++`GIT_NO_LAZY_FETCH` to `0`, but only for the matching repositories:
    ++unlike that environment variable, the trust is not inherited by child
    ++processes. So if a trusted repository lazily fetches from a promisor
    ++remote that is itself served by `upload-pack` on the same machine,
    ++for example through a local path or a `file://` URL, lazy fetching is
    ++allowed there only if that promisor remote is also listed here. An
     +explicitly set `GIT_NO_LAZY_FETCH` takes precedence over this setting.
     ++
     +Note that this allows lazy fetching from any promisor remote
    @@ Documentation/config/uploadpack.adoc: uploadpack.allowRefInWant::
     ++
     +As this is a multi-valued setting, you can add more than one
     +repository via `git config (--global|--system) --add`. To reset the
    -+list of trusted repositories (e.g. to override any such repositories
    ++list of trusted repositories (e.g., to override any such repositories
     +specified in the system config), add an `uploadpack.lazyFetchTrusted`
     +entry with an empty value.
     ++
    @@ Documentation/config/uploadpack.adoc: uploadpack.allowRefInWant::
     
      ## Documentation/git-upload-pack.adoc ##
     @@ Documentation/git-upload-pack.adoc: This is implemented by having `upload-pack` internally set the
    + `GIT_NO_LAZY_FETCH` variable to `1`. If you want to override it
      (because you are fetching from a partial clone, and you are sure
      you trust it), you can explicitly set `GIT_NO_LAZY_FETCH` to
    - `0`.
    +-`0`.
    ++`0`. As it is an environment variable, it is also inherited by child
    ++processes, including any `upload-pack` run to lazily fetch from a
    ++promisor remote on the same machine.
     ++
     +Instead of setting `GIT_NO_LAZY_FETCH` to `0` in the environment, a
     +server operator can allow lazy fetching on a per-repository basis by
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
     +'
     +
     +test_expect_success "uploadpack.lazyFetchTrusted needs the git dir of a non-bare repo" '
    -+	test_when_finished "rm -rf nonbare client client2" &&
    ++	test_when_finished "rm -rf nonbare nonbare-pack-* client client2" &&
     +
     +	# Create a non-bare repo, without any worktree content, so that
     +	# its largest object can be filtered out below
    @@ t/t5710-promisor-remote-capability.sh: test_expect_success "clone with promisor.
     +	git clone --no-local --filter="blob:limit=1k" nonbare client2 &&
     +	check_missing_objects nonbare 0 ""
     +'
    ++
    ++test_expect_success "uploadpack.lazyFetchTrusted needs the git dir a .git file points to" '
    ++	test_when_finished "rm -rf sepwt sepgit sep-pack-* client client2" &&
    ++
    ++	# Create a non-bare repo with a ".git" file pointing to a
    ++	# separate git dir, without any worktree content, so that its
    ++	# largest object can be filtered out below
    ++	git init --separate-git-dir="$(pwd)/sepgit" sepwt &&
    ++	test_path_is_file sepwt/.git &&
    ++	git -C sepwt remote add origin "$TRASH_DIRECTORY_URL/template" &&
    ++	git -C sepwt fetch origin &&
    ++	git -C sepwt update-ref HEAD FETCH_HEAD &&
    ++
    ++	git -C sepwt remote add lop "$TRASH_DIRECTORY_URL/lop" &&
    ++	git -C sepwt config remote.lop.promisor true &&
    ++	git -C sepwt config uploadpack.allowFilter true &&
    ++	git -C sepwt config uploadpack.allowAnySHA1InWant true &&
    ++	git -C sepwt config promisor.advertise false &&
    ++
    ++	# Repack everything, then repack without the largest object and
    ++	# create a promisor pack, like initialize_server() does
    ++	git -C sepwt -c repack.writebitmaps=false repack -a -d &&
    ++	rm -f sepgit/objects/pack/*.promisor &&
    ++	git -C sepwt -c repack.writebitmaps=false repack -a -d \
    ++		--filter=blob:limit=5k --filter-to="$(pwd)/sep-pack" &&
    ++	promisor_file=$(ls sepgit/objects/pack/*.pack | sed "s/\.pack/.promisor/") &&
    ++	>"$promisor_file" &&
    ++	check_missing_objects sepwt 1 "$oid" &&
    ++
    ++	# The ".git" file does not identify the repo, so it is not
    ++	# trusted and the clone fails
    ++	test_config_global uploadpack.lazyFetchTrusted "$(pwd)/sepwt/.git" &&
    ++	test_must_fail git clone --no-local --filter="blob:limit=1k" \
    ++		sepwt client 2>err &&
    ++	test_grep "lazy fetching disabled" err &&
    ++	check_missing_objects sepwt 1 "$oid" &&
    ++
    ++	# The git dir the ".git" file points to identifies the repo, so
    ++	# it is trusted and the clone succeeds
    ++	test_config_global uploadpack.lazyFetchTrusted "$(pwd)/sepgit" &&
    ++	git clone --no-local --filter="blob:limit=1k" sepwt client2 &&
    ++	check_missing_objects sepwt 0 ""
    ++'
    ++
    ++test_expect_success "uploadpack.lazyFetchTrusted trust does not propagate to promisor remotes" '
    ++	# No promisors are advertised
    ++	git -C server config promisor.advertise false &&
    ++	test_when_finished "rm -rf client lop2" &&
    ++
    ++	# Create "lop2", a partial clone that is also missing the
    ++	# largest object, and that can lazily fetch it from "lop"
    ++	test_config -C template uploadpack.allowFilter true &&
    ++	git clone --bare --no-local --filter="blob:limit=5k" \
    ++		"$TRASH_DIRECTORY_URL/template" lop2 &&
    ++	git -C lop2 remote set-url origin "$TRASH_DIRECTORY_URL/lop" &&
    ++	git -C lop2 config uploadpack.allowFilter true &&
    ++	git -C lop2 config uploadpack.allowAnySHA1InWant true &&
    ++	check_missing_objects lop2 1 "$oid" &&
    ++
    ++	# Make "lop2" the only promisor remote of the server. Note that
    ++	# "remote.lop.partialCloneFilter" also makes "lop" a promisor
    ++	# remote, so it has to be unset too.
    ++	git -C server remote add lop2 "$TRASH_DIRECTORY_URL/lop2" &&
    ++	git -C server config remote.lop2.promisor true &&
    ++	test_when_finished "git -C server remote remove lop2" &&
    ++	git -C server config --unset remote.lop.promisor &&
    ++	test_when_finished "git -C server config remote.lop.promisor true" &&
    ++	lop_filter="$(git -C server config remote.lop.partialCloneFilter)" &&
    ++	git -C server config --unset remote.lop.partialCloneFilter &&
    ++	test_when_finished "git -C server config remote.lop.partialCloneFilter \"$lop_filter\"" &&
    ++
    ++	# Only the server is trusted, not "lop2", so the upload-pack
    ++	# serving "lop2" to the server refuses to lazily fetch from "lop"
    ++	test_config_global uploadpack.lazyFetchTrusted "$(pwd)/server" &&
    ++	test_must_fail git clone --no-local --filter="blob:limit=5k" \
    ++		server client 2>err &&
    ++	test_grep "lazy fetching disabled" err &&
    ++	check_missing_objects server 1 "$oid" &&
    ++	check_missing_objects lop2 1 "$oid" &&
    ++
    ++	# Once "lop2" is also trusted, the clone succeeds
    ++	git config --global --add uploadpack.lazyFetchTrusted "$(pwd)/lop2" &&
    ++	git clone --no-local --filter="blob:limit=5k" server client &&
    ++	check_missing_objects server 0 "" &&
    ++
    ++	# Reinitialize server so that the largest object is missing again
    ++	initialize_server 1 "$oid"
    ++'
     +
      test_expect_success "init + fetch with promisor.advertise set to 'true'" '
      	git -C server config promisor.advertise true &&

Christian Couder (5):
  promisor-remote: factor out lazy_fetch_objects()
  setup: extract path_allowlist_apply()
  upload-pack: read uploadpack.lazyFetchTrusted
  promisor-remote: prevent infinite recursion when lazy fetching
  builtin/upload-pack: don't disable lazy fetching on trusted repo

 Documentation/config/uploadpack.adoc  |  57 +++++++
 Documentation/git-upload-pack.adoc    |   9 +-
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  19 ++-
 environment.h                         |   8 +
 promisor-remote.c                     | 105 ++++++++----
 setup.c                               | 142 ++++++++++------
 setup.h                               |  50 ++++++
 t/t0410-partial-clone.sh              |  33 ++++
 t/t5710-promisor-remote-capability.sh | 230 ++++++++++++++++++++++++++
 upload-pack.c                         |  59 +++++++
 upload-pack.h                         |   3 +
 12 files changed, 639 insertions(+), 80 deletions(-)

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4

In "promisor-remote.c:fetch_objects()", there is a check to disable lazy fetching when the `GIT_NO_LAZY_FETCH` environment variable is set. The fetch_objects() function is called once per promisor remote though. So the check might be performed more times than necessary.

Also promisor_remote_get_direct() mixes up the logic deciding which promisor remotes to try with the logic checking that the objects that could not be fetched are promisor objects.

Let's refactor the lazy fetching logic out of these two functions into a new lazy_fetch_objects() function.

This is a pure refactoring with no intended behavior change. Two things shift in ways that are observably equivalent though:

  - the `GIT_NO_LAZY_FETCH` check is now performed once up front,
    instead of once per promisor remote, and

  - promisor_remote_init() is no longer called when lazy fetching
    is disabled.

The latter is fine because the convention around promisor_remote_init() is that whoever needs to access the promisor remote information is expected to initialize it beforehand, and not that it should be initialized once at the very beginning before doing random things on promisor remotes. So moving its call site into lazy_fetch_objects(), which is the only code that needs the promisor remotes here, follows that convention. Nothing downstream of it, like is_promisor_object(), needs it when lazy fetching is disabled.

While at it, let's document try_promisor_remotes() and the new lazy_fetch_objects() function, especially how their `remaining_oids`, `remaining_nr` and `to_free` arguments are used, as the ownership rules around `to_free` are easy to get wrong.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 promisor-remote.c | 83 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 57 insertions(+), 26 deletions(-)

In a following commit we are going to check whether a repository is part of an allowlist specified in a config variable.

To prepare for that let's extract existing code from safe_directory_cb() into a new path_allowlist_apply() helper that will help with such checks.

While at it let's make the helper's code simpler and more generic, by passing it a `bool (*allow_path)(const char *path, void *cbdata)` function that decides if a path is acceptable by the caller, and let's add a NEEDSWORK comment about it silently ignoring missing, possibly misspelled, paths.

To further simplify how to reuse that new helper, and avoid duplicating the config-value handling in a future commit, let's also introduce a path_allowlist_config_apply() helper.

As the new path_allowlist_apply() function reports its result through a `bool *matches` argument, let's also change the `int is_safe` member of `struct safe_directory_data` to a `bool`, so that its address can be passed as that argument.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 setup.c | 142 +++++++++++++++++++++++++++++++++++++-------------------
 setup.h |  50 ++++++++++++++++++++
 2 files changed, 143 insertions(+), 49 deletions(-)

Previous commits created and prepared the path_allowlist_apply() and path_allowlist_config_apply() functions, but used them only for the "safe.directory" configuration variable.

Let's reuse these functions for a new "uploadpack.lazyFetchTrusted" configuration variable.

It allows us to:

  - read an allowlist from that config variable,
  - check if the current repo is in that list, and
  - return the result from a new upload_pack_lazy_fetch_trusted()
    function.

As path_allowlist_config_apply() lets each caller decide which paths it is willing to accept using a callback, let's pass it a new allow_trusted_path() callback. Unlike the "safe.directory" callback, it accepts only absolute paths, and not ".", as `upload-pack` always serves a repository given by an absolute path, so there is no "current repository" for "." to refer to.

Note that a served repository is identified by its git directory, and not by its worktree. This is because `upload-pack` uses enter_repo() instead of the usual repository discovery, so it never learns about a worktree and `r->worktree` is always NULL there. In practice this means that a non-bare repository served as "/srv/repo" has to be allowlisted as "/srv/repo/.git" (or as the directory its ".git" file points to, if it has a ".git" file instead of a ".git" directory).

The new upload_pack_lazy_fetch_trusted() function will be used in a following commit.

Note that the new config variable should be read only from protected configuration files.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 upload-pack.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 upload-pack.h |  3 +++
 2 files changed, 62 insertions(+)

If a repository R is configured to lazy fetch from a promisor remote P which is also configured to in turn lazy fetch from R, there is an infinite recursion: R asks P for a missing object, P asks R for it, and so on. The simplest case of this is a repository configured as its own promisor remote.

This is not reachable when serving a repository by default, as `upload-pack` sets `GIT_NO_LAZY_FETCH` to 1, which makes the nested `upload-pack` refuse to lazily fetch. A following commit will let server operators allow lazy fetching for repositories they trust though, and as `GIT_NO_LAZY_FETCH` is then set to 0 and passed down to child processes, nothing stops the recursion anymore.

It does not recurse forever in practice, but only because each level adds one more variable to the environment of the child process, so after a while `exec()` fails with:

    fatal: cannot exec 'git-upload-pack ...': Argument list too long
    fatal: unable to fork

To avoid this pathological case altogether, let's use a new `GIT_INTERNAL_LAZY_FETCH_DEPTH` to count the recursion depth, and let's check that it doesn't exceed a MAX_LAZY_FETCH_DEPTH limit (set to 5 for now).

Note that some nesting is legitimate: when `git fetch` runs `index-pack`, it can lazily fetch REF_DELTA bases that are missing locally, so the limit should not be 1.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 environment.h            |  8 ++++++++
 promisor-remote.c        | 26 ++++++++++++++++++++++----
 t/t0410-partial-clone.sh | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 4 deletions(-)

A previous commit added a new "uploadpack.lazyFetchTrusted" protected config variable that can contain an allowlist of repos, as well as functions to check if the current repo is in that list. But when the current repo is in that list, we currently do nothing.

Since 7b70e9efb1 (upload-pack: disable lazy-fetching by default, 2024-04-16), `upload-pack` sets `GIT_NO_LAZY_FETCH` to 1 itself, unconditionally, because by default it shouldn't trust the repositories it serves. Lazily fetching runs `git fetch`, which may execute arbitrary commands specified in the configuration and hooks of the served repo.

The new "uploadpack.lazyFetchTrusted" protected config variable is not about overriding an environment variable. It's rather about teaching the code that automatically sets `GIT_NO_LAZY_FETCH` (because it had no way to know if the served repo could be trusted) to look at the new config variable to find out if a server operator actually vouched for that repo.

Let's implement that, so we now have the following cases:

  - if `GIT_NO_LAZY_FETCH` is already set, we honor it and leave it
    alone, as it comes from the server operator,

  - otherwise, if the served repo is in the
    "uploadpack.lazyFetchTrusted" allowlist, we don't disable lazy
    fetching,

  - otherwise, we disable lazy fetching, as we used to.

This allows `upload-pack` and its `pack-objects` child process to lazily fetch the objects they need to serve a client, for example when the filter used by the client and the one used by the server don't match.

Note that what a server operator vouches for by listing a repo there is that the promisor remotes this repo is configured to lazily fetch from, as well as its configuration and hooks, are trustworthy. Whether a client trusts the repo it fetches from is a separate matter, and up to the client.

As `pack-objects`, which performs the lazy fetch when serving a client, is a child process of `upload-pack`, not setting `GIT_NO_LAZY_FETCH` in `upload-pack` is enough for it to be allowed to lazily fetch, without any further plumbing.

On the other hand, as we leave `GIT_NO_LAZY_FETCH` unset for a trusted repo instead of setting it to 0, the trust doesn't propagate: if a trusted repo lazily fetches from a promisor remote that is itself served by `upload-pack` on the same machine, that nested `upload-pack` decides for its own repo. This is unlike when a server operator sets `GIT_NO_LAZY_FETCH` to 0, as that is inherited by all child processes.

Now that "uploadpack.lazyFetchTrusted" is actually doing something, let's document it (including this difference with `GIT_NO_LAZY_FETCH`), let's reference it from `GIT_NO_LAZY_FETCH`'s docs, and let's add tests for it.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 Documentation/config/uploadpack.adoc  |  57 +++++++
 Documentation/git-upload-pack.adoc    |   9 +-
 Documentation/git.adoc                |   4 +-
 builtin/upload-pack.c                 |  19 ++-
 t/t5710-promisor-remote-capability.sh | 230 ++++++++++++++++++++++++++
 5 files changed, 316 insertions(+), 3 deletions(-)

On Tue, Sep 29, 2026 at 7:47 PM Junio C Hamano <gitster@pobox.com> wrote:

Yeah, "each value of which specifies" is used in the v5 I just sent.

"from which" would not be quite right, because the client would lazily fetch from the promisor remotes of this server repo (using the "promisor-remote" capability), not directly from this repo. So the rest of the sentence hasn't changed in v5.

Applied in v5.

Thanks!

On Tue, Sep 29, 2026 at 7:26 PM Junio C Hamano <gitster@pobox.com> wrote:

There is the following new NEEDSWORK comment in the v5 I just sent:

+        * NEEDSWORK: this also silently ignores misspelled paths. We
+        * may want to warn about a missing path unless it is marked
+        * as allowed to be missing, e.g., with an ":(optional)"
+        * prefix like pathname-typed configuration values, and hint
+        * about that prefix in the warning.

Thanks.

On Fri, Oct 2, 2026 at 10:57 AM Christian Couder <christian.couder@gmail.com> wrote:

Actually "from which" would not be quite right, but not for the reason I just gave. Sorry. It is not about the client, nor about the "promisor-remote" capability.

"uploadpack.lazyFetchTrusted" controls server-side lazy fetching. So when a repo is listed in that config option, the server's `upload-pack` (via `pack-objects`), while serving that repo, lazily fetches missing objects _for_ that repo (not from it).

Christian Couder <christian.couder@gmail.com> writes:

I was hoping to see more substantial reviews from others (compared to my rather nitpicky review on v4), but nobody has bitten yet. Shall we declare that we have reached the point of diminishing returns and mark the topic for 'next'?

Thanks.