dir: Try to delete the remote if we failed to add it entirely by swick · Pull Request #6547 · flatpak/flatpak

swick · 2026-03-25T17:37:18Z

Ideally, we would be able to atomically add and remove remotes, but we're very far from that ideal state. The current behavior is really suboptimal and leaves the remotes in a inconsistent state if initialization failed. We can at least make it better by trying to clean up the half-initialized mess we're currently in. It does however not protect against SIGKILL-like aborts, as that would require it to be atomic.

Closes: #6449

swick · 2026-04-13T15:01:36Z

There is an awkward thing remaining: if the gpg "key" has size of 0 (e.g. an empty file) then the system helper sees this as "no key was requested". I'll file that under future improvement because that has already been the case and doesn't impact the problem we want to solve here which is mainly failure to import a GPG key because of the time of the system being wrong.

swick · 2026-04-13T15:02:05Z

/cc @AdrianVovk. Don't know who else to ping.

craftyguy · 2026-04-13T21:42:34Z

Unfortunately this doesn't seem to solve it for me:

foo:~$ date
 Thu Jan  1 01:19:02 PST 1970


foo:~$ G_MESSAGES_DEBUG=all flatpak -vv remote-add --system --from flathub
/usr/share/flatpak/remotes.d/flathub.flatpakrepo
(flatpak remote-add:3365): GLib-DEBUG: 00:17:16.956: setenv()/putenv() are not thread-safe and should not be used after threads are created
(flatpak remote-add:3365): GLib-GIO-DEBUG: 00:17:16.967: _g_io_module_get_default: Found default implementation local (GLocalVfs) for ‘gio-vfs’
(flatpak remote-add:3365): GLib-DEBUG: 00:17:16.979: unsetenv() is not thread-safe and should not be used after threads are created
F: Calling system helper: EnsureRepo
==== AUTHENTICATING FOR org.freedesktop.Flatpak.modify-repo ====
Authentication is required to modify a system repository
Authenticating as: clayton
Password:
==== AUTHENTICATION COMPLETE ====
error: GPG: Unable to export keys: GPGME: No data
foo:~$ cat /var/lib/flatpak/repo/config
[core]
repo_version=1
mode=bare-user-only
min-free-space-size=500MB
xa.applied-remotes=flathub;

[remote "flathub"]
url=https://dl.flathub.org/repo/
xa.title=Flathub
gpg-verify=true
gpg-verify-summary=true
xa.comment=Central repository of Flatpak applications
xa.description=Central repository of Flatpak applications
xa.icon=https://dl.flathub.org/repo/logo.svg
xa.homepage=https://flathub.org/
foo:~$ ls /var/lib/flatpak/repo
extensions  objects  refs  state  tmp  config

It seems like this patch doesn't fix the apply_new_flatpakrepo path, which is taken when the system helper auto-applies preconfigured remotes in /usr/share/flatpak/remotes.d/ (IIUC)

I managed to come up with a patch that fixes this for me:

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index e381b16d..2311c498 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -4088,8 +4088,25 @@ apply_new_flatpakrepo (const char *remote_name,
 
       if (!ostree_repo_remote_gpg_import (repo, remote_name, input_stream,
                                           NULL, &imported, NULL, error))
-        return FALSE;
-
+        {
+          /* The config and xa.applied-remotes were already written above, so we
+           * need to undo that here to avoid leaving the remote in a broken state
+           * where it is marked as applied but has no GPG key imported.
+           * ostree_repo_remote_delete() handles removing the remote group and
+           * keyring, after which we strip it from xa.applied-remotes and
+           * write the config back. */
+          ostree_repo_remote_delete (repo, remote_name, NULL, NULL);
+          g_autoptr(GKeyFile) cleanup_config = ostree_repo_copy_config (repo);
+          g_auto(GStrv) applied = g_key_file_get_string_list (cleanup_config, "core", "xa.applied-remotes", NULL, NULL);
+          g_autoptr(GPtrArray) new_applied = g_ptr_array_new_with_free_func (g_free);
+          for (int j = 0; applied != NULL && applied[j] != NULL; j++)
+            if (g_strcmp0 (applied[j], remote_name) != 0)
+              g_ptr_array_add (new_applied, g_strdup (applied[j]));
+          g_key_file_set_string_list (cleanup_config, "core", "xa.applied-remotes",
+                                      (const char * const *) new_applied->pdata, new_applied->len);
+          ostree_repo_write_config (repo, cleanup_config, NULL);
+          return FALSE;
+        }
       g_info ("Imported %u GPG key%s to remote \"%s\"", imported, (imported == 1) ? "" : "s", remote_name);
     }

When import fails, no repo config is added:

foo:~$ date
 Thu Jan  1 01:19:02 PST 1970
foo:~$ G_MESSAGES_DEBUG=all flatpak -vv remote-add --system --from flathub
/usr/share/flatpak/remotes.d/flathub.flatpakrepo
(flatpak remote-add:7095): GLib-DEBUG: 01:19:09.326: setenv()/putenv() are not thread-safe and should not be used after threads are created
(flatpak remote-add:7095): GLib-GIO-DEBUG: 01:19:09.337: _g_io_module_get_default: Found default implementation local (GLocalVfs) for ‘gio-vfs’
(flatpak remote-add:7095): GLib-DEBUG: 01:19:09.348: unsetenv() is not thread-safe and should not be used after threads are created
(flatpak remote-add:7095): OSTree-DEBUG: 01:19:09.357: using fuse: 0
F: Calling system helper: EnsureRepo
==== AUTHENTICATING FOR org.freedesktop.Flatpak.modify-repo ====
Authentication is required to modify a system repository
Authenticating as: clayton
Password:
==== AUTHENTICATION COMPLETE ====
error: GPG: Unable to export keys: GPGME: No data
foo:~$ cat /var/lib/flatpak/repo/config
[core]
repo_version=1
mode=bare-user-only
min-free-space-size=500MB
xa.applied-remotes=
foo:~$ ls /var/lib/flatpak/repo/
config      extensions  objects     refs        state       tmp

idk if there's some helper function this patch could use to avoid doing some of the manual cleanup there...

swick · 2026-04-14T18:46:57Z

Managed to add a test for that scenario as well, and fixed it with a slightly different approach.

craftyguy · 2026-04-14T18:56:33Z

Oh yeah this is MUCH nicer, I'm clearly not familiar with libostree 😅

Thanks a lot for working on this, I'll test out this patch!

craftyguy · 2026-04-14T22:06:19Z

@swick works great now! When the system date is wrong (e.g. first boot, RTC not correct, no inet), remotes in /usr/share/flatpak/remotes.d are not added at all by the helper. Once the system is online and date is ntp synced, helper adds remotes and everything works fine. No half-added broken remote config :)

Ideally, we would be able to atomically add and remove remotes, but we're very far from that ideal state. The current behavior is really suboptimal and leaves the remotes in a inconsistent state if initialization failed. We can at least make it better by trying to clean up the half-initialized mess we're currently in. It does however not protect against SIGKILL-like aborts, as that would require it to be atomic. Closes: flatpak#6449 Co-authored-by: craftyguy "Clayton Craft" <[email protected]>

swick force-pushed the wip/remote-add-failure-add branch from f8ffe74 to 1cebf01 Compare April 13, 2026 14:58

swick force-pushed the wip/remote-add-failure-add branch from 1cebf01 to 8020b81 Compare April 14, 2026 18:46

bbhtt reviewed Apr 16, 2026

View reviewed changes

Comment thread common/flatpak-dir.c

swick force-pushed the wip/remote-add-failure-add branch from 8020b81 to 84c7fe7 Compare April 16, 2026 11:09

swick added this to the 1.18 milestone Apr 16, 2026

bbhtt approved these changes Apr 18, 2026

View reviewed changes

swick added this pull request to the merge queue Apr 20, 2026

Merged via the queue into flatpak:main with commit 4364233 Apr 20, 2026
11 checks passed

swick deleted the wip/remote-add-failure-add branch April 20, 2026 14:07

Navigation Menu

Repository navigation

dir: Try to delete the remote if we failed to add it entirely - #6547

dir: Try to delete the remote if we failed to add it entirely#6547
swick merged 1 commit into
flatpak:mainfrom
swick:wip/remote-add-failure-add

swick commented Mar 25, 2026

Uh oh!

swick commented Apr 13, 2026

Uh oh!

swick commented Apr 13, 2026

Uh oh!

craftyguy commented Apr 13, 2026

Uh oh!

swick commented Apr 14, 2026

Uh oh!

craftyguy commented Apr 14, 2026

Uh oh!

craftyguy commented Apr 14, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

swick commented Mar 25, 2026

Uh oh!

swick commented Apr 13, 2026

Uh oh!

swick commented Apr 13, 2026

Uh oh!

craftyguy commented Apr 13, 2026

Uh oh!

swick commented Apr 14, 2026

Uh oh!

craftyguy commented Apr 14, 2026

Uh oh!

craftyguy commented Apr 14, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants