* [PATCH] promisor-remote: fix crash without a repository
@ 2026-11-13 0:06 Christian Couder
2026-11-13 19:42 ` Junio C Hamano
0 siblings, 1 reply; 2+ messages in thread
From: Christian Couder @ 2026-11-13 0:06 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Christian Couder
When `git ls-remote` is run outside a repo with some promisor remote
related config options that make it write to its config, namely:
- "promisor.acceptFromServerUrl", which makes it auto-create a
remote, or
- "promisor.acceptFromServer" with "promisor.storeFields", which
makes it store fields,
it crashes in configure_auto_promisor_remote() or store_one_field(), as
the `repo` argument passed to these functions has its `gitdir` member
set to NULL.
This has been happening since 3e20258d11 (promisor-remote: allow a
client to store fields, 2026-02-16) for the `storeFields` case, and
since 7a56394fc6 (promisor-remote: auto-configure unknown remotes,
2026-05-27) for the auto-creation case.
Let's fix that by checking if `repo->gitdir` is NULL at the beginning
of filter_promisor_remote() and returning from it right away in that
case. It doesn't make sense to further process the promisor remote
advertisement as `git ls-remote` currently has no use for promisor
remotes anyway.
Note that `git ls-remote` seems to be the only command that can run
outside a repo and still process the "promisor-remote" capability
advertised by the server, as sending the `ls-refs` protocol v2
command makes it call send_capabilities(), which processes that
advertisement.
Let's also add tests in t5710 for both cases where a crash happened.
Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
This is a fix for another bug found during the NVIDIA hackathon
following the Gerrit User Summit 2026.
promisor-remote.c | 7 +++++-
t/t5710-promisor-remote-capability.sh | 31 +++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/promisor-remote.c b/promisor-remote.c
index 316d9950aa..9c405847bb 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -1310,9 +1310,14 @@ static void filter_promisor_remote(struct repository *repo,
struct store_info *store_info = NULL;
struct string_list_item *item;
bool reload_config = false;
- enum accept_promisor accept = accept_from_server(repo);
+ enum accept_promisor accept;
struct string_list accept_urls = STRING_LIST_INIT_DUP;
+ if (!repo->gitdir)
+ return;
+
+ accept = accept_from_server(repo);
+
/* Load and validate the acceptFromServerUrl config */
load_accept_from_server_url(repo, &accept_urls);
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 463e9e00b0..50a73ec8b0 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -813,6 +813,37 @@ test_expect_success "clone with invalid promisor.acceptFromServerUrl" '
initialize_server 1 "$oid"
'
+test_expect_success "ls-remote without a repo doesn't auto-configure" '
+ git -C server config promisor.advertise true &&
+
+ nongit git -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
+ ls-remote "$TRASH_DIRECTORY_URL/server" >out 2>&1 &&
+
+ # Refs are properly listed
+ test_grep "refs/heads/" out &&
+
+ # No remote is configured
+ test_grep ! "Auto-creating" out
+'
+
+test_expect_success "ls-remote without a repo doesn't store fields" '
+ git -C server config promisor.advertise true &&
+ test_config -C server promisor.sendFields token &&
+ test_config -C server remote.lop.token "fooBar" &&
+
+ nongit git -c promisor.acceptFromServer=All \
+ -c promisor.storeFields=token \
+ -c remote.lop.promisor=true \
+ -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+ ls-remote "$TRASH_DIRECTORY_URL/server" >out 2>&1 &&
+
+ # Refs are properly listed
+ test_grep "refs/heads/" out &&
+
+ # No field is stored
+ test_grep ! "Storing new token" out
+'
+
test_expect_success "clone with promisor.sendFields" '
git -C server config promisor.advertise true &&
test_when_finished "rm -rf client" &&
base-commit: 1891de46f79dc444a47db1c87bd0c8c76b5463f2
--
2.56.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] promisor-remote: fix crash without a repository
2026-11-13 0:06 [PATCH] promisor-remote: fix crash without a repository Christian Couder
@ 2026-11-13 19:42 ` Junio C Hamano
0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2026-11-13 19:42 UTC (permalink / raw)
To: Christian Couder; +Cc: git, Patrick Steinhardt
Christian Couder <christian.couder@gmail.com> writes:
> When `git ls-remote` is run outside a repo with some promisor remote
> related config options that make it write to its config, namely:
>
> - "promisor.acceptFromServerUrl", which makes it auto-create a
> remote, or
> - "promisor.acceptFromServer" with "promisor.storeFields", which
> makes it store fields,
>
> it crashes in configure_auto_promisor_remote() or store_one_field(), as
> the `repo` argument passed to these functions has its `gitdir` member
> set to NULL.
It is tricky that "ls-remote" wants to work outside a repository.
> This has been happening since 3e20258d11 (promisor-remote: allow a
> client to store fields, 2026-02-16) for the `storeFields` case, and
> since 7a56394fc6 (promisor-remote: auto-configure unknown remotes,
> 2026-05-27) for the auto-creation case.
>
> Let's fix that by checking if `repo->gitdir` is NULL at the beginning
> of filter_promisor_remote() and returning from it right away in that
> case. It doesn't make sense to further process the promisor remote
> advertisement as `git ls-remote` currently has no use for promisor
> remotes anyway.
Another possibility is to nip this higher in the callchain. If we
know, as you said, "ls-remote" has no need to talk about promisors,
turning promisor_remote_reply() into a NOOP outside a repository
might be better, but the function does not take a repository
parameter to be able to tell if we have a repository in the first
place---perhaps we should add one? An illustration patch below.
The callers (there are only two) should of course be updated.
In any case, we should comment and say why we punt when '.gitdir' is
NULL and why it is safe to punt. In the illustration patch below,
I only covered the former but not the latter.
promisor-remote.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git c/promisor-remote.c w/promisor-remote.c
index 316d9950aa..702c53c2c4 100644
--- c/promisor-remote.c
+++ w/promisor-remote.c
@@ -1373,11 +1373,20 @@ static void filter_promisor_remote(struct repository *repo,
}
}
-void promisor_remote_reply(const char *info, char **accepted_out)
+void promisor_remote_reply(struct repository *repo, const char *info, char **accepted_out)
{
struct string_list accepted_remotes = STRING_LIST_INIT_NODUP;
- filter_promisor_remote(the_repository, &accepted_remotes, info);
+ /*
+ * "ls-remote" has no business talking about promisors, and worse yet,
+ * it can be run outside a repository.
+ */
+ if (!repo->gitdir) {
+ if (accepted_out)
+ *accepted_out = NULL;
+ return;
+ }
+ filter_promisor_remote(repo, &accepted_remotes, info);
if (accepted_out) {
if (accepted_remotes.nr) {
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-13 19:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-13 0:06 [PATCH] promisor-remote: fix crash without a repository Christian Couder
2026-11-13 19:42 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox