Git development
 help / color / mirror / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	Patrick Steinhardt <ps@pks.im>,
	Karthik Nayak <karthik.188@gmail.com>, Jeff King <peff@peff.net>,
	Elijah Newren <newren@gmail.com>,
	Christian Couder <christian.couder@gmail.com>
Subject: [PATCH 0/5] Introduce 'uploadpack.lazyFetchTrusted'
Date: Fri,  7 Aug 2026 15:55:06 +0200	[thread overview]
Message-ID: <20260807135511.1818458-1-christian.couder@gmail.com> (raw)
In-Reply-To: <20260710085137.4171240-1-christian.couder@gmail.com>

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(-)

-- 
2.55.0.530.gdb3615d990.dirty


  parent reply	other threads:[~2026-08-07 13:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10  8:51 [PATCH 0/3] Introduce a 'fromAccepted' option to GIT_NO_LAZY_FETCH Christian Couder
2026-07-10  8:51 ` [PATCH 1/3] promisor-remote: factor out lazy_fetch_objects() Christian Couder
2026-07-10  8:51 ` [PATCH 2/3] promisor-remote: introduce enum allow_lazy_fetch Christian Couder
2026-07-10  8:51 ` [PATCH 3/3] promisor-remote: teach 'fromAccepted' to GIT_NO_LAZY_FETCH Christian Couder
2026-07-10 19:50 ` [PATCH 0/3] Introduce a 'fromAccepted' option " brian m. carlson
2026-07-12  9:06   ` Christian Couder
2026-08-07 13:55 ` Christian Couder [this message]
2026-08-07 13:55   ` [PATCH 1/5] promisor-remote: factor out lazy_fetch_objects() Christian Couder
2026-08-07 13:58     ` Christian Couder
2026-08-07 13:55   ` [PATCH 2/5] setup: extract path_allowlist_apply() Christian Couder
2026-08-07 13:55   ` [PATCH 3/5] setup: add 'allow_dot' arg to path_allowlist_apply() Christian Couder
2026-08-07 13:55   ` [PATCH 4/5] upload-pack: read uploadpack.lazyFetchTrusted Christian Couder
2026-08-07 13:55   ` [PATCH 5/5] builtin/upload-pack: set GIT_NO_LAZY_FETCH to 0 on trusted repo Christian Couder
2026-08-07 18:31   ` [PATCH 0/5] Introduce 'uploadpack.lazyFetchTrusted' Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260807135511.1818458-1-christian.couder@gmail.com \
    --to=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox