mirror/flatpak
1
0
Fork 0
mirror of https://github.com/flatpak/flatpak.git synced 2026-05-11 17:36:44 -04:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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
Co-authored-by: craftyguy "Clayton Craft" <clayton@craftyguy.net>
This commit is contained in:
Sebastian Wick
2026-03-25 18:30:10 +01:00
parent 7781da7767
commit 43642337e4
2 changed files with 93 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

75
common/flatpak-dir.c
View File

@@ -4618,10 +4618,12 @@ apply_new_flatpakrepo (const char *remote_name,
g_autoptr(GKeyFile) group_config = NULL;
g_autoptr(GKeyFile) keyfile = g_key_file_new ();
g_autoptr(GError) local_error = NULL;
g_autoptr(GKeyFile) old_config = NULL;
g_autoptr(GKeyFile) new_config = NULL;
g_auto(GStrv) old_applied_remotes = NULL;
g_autoptr(GPtrArray) new_applied_remotes = NULL;
int i;
gboolean res = FALSE;
if (!g_key_file_load_from_file (keyfile, flatpak_file_get_path_cached (file), 0, &local_error))
{
@@ -4636,6 +4638,7 @@ apply_new_flatpakrepo (const char *remote_name,
return FALSE;
}
old_config = ostree_repo_copy_config (repo);
new_config = ostree_repo_copy_config (repo);
old_applied_remotes = g_key_file_get_string_list (new_config, "core", "xa.applied-remotes", NULL, NULL);
@@ -4652,10 +4655,10 @@ apply_new_flatpakrepo (const char *remote_name,
(const char * const *) new_applied_remotes->pdata, new_applied_remotes->len);
if (!ostree_repo_write_config (repo, new_config, error))
return FALSE;
goto out;
if (!ostree_repo_reload_config (repo, NULL, error))
return FALSE;
goto out;
if (gpg_data != NULL)
{
@@ -4664,12 +4667,21 @@ apply_new_flatpakrepo (const char *remote_name,
if (!ostree_repo_remote_gpg_import (repo, remote_name, input_stream,
NULL, &imported, NULL, error))
return FALSE;
goto out;
g_info ("Imported %u GPG key%s to remote \"%s\"", imported, (imported == 1) ? "" : "s", remote_name);
}
return TRUE;
res = TRUE;
out:
if (!res)
{
/* Roll back the changes. Ideally they would be atomic, because if the
* program terminates before we roll back, we end up in a broken state */
ostree_repo_write_config (repo, old_config, NULL);
ostree_repo_reload_config (repo, NULL, NULL);
}
return res;
}
static gboolean
@@ -16002,9 +16014,11 @@ flatpak_dir_modify_remote (FlatpakDir *self,
g_autofree char *group = g_strdup_printf ("remote \"%s\"", remote_name);
g_autofree char *url = NULL;
g_autofree char *metalink = NULL;
g_autoptr(GKeyFile) old_config = NULL;
g_autoptr(GKeyFile) new_config = NULL;
g_autofree gchar *filter_path = NULL;
gboolean has_remote;
gboolean res = FALSE;
if (strchr (remote_name, '/') != NULL)
return flatpak_fail_error (error, FLATPAK_ERROR_REMOTE_NOT_FOUND, _("Invalid character '/' in remote name: %s"),
@@ -16058,6 +16072,8 @@ flatpak_dir_modify_remote (FlatpakDir *self,
if (!flatpak_dir_cleanup_remote_for_url_change (self, remote_name, url, cancellable, error))
return FALSE;
old_config = ostree_repo_copy_config (self->repo);
/* Add it if its not there yet */
if (!ostree_repo_remote_change (self->repo, NULL,
OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS,
@@ -16070,21 +16086,7 @@ flatpak_dir_modify_remote (FlatpakDir *self,
copy_remote_config (new_config, config, remote_name);
if (!ostree_repo_write_config (self->repo, new_config, error))
return FALSE;
if (gpg_data != NULL)
{
g_autoptr(GInputStream) input_stream = g_memory_input_stream_new_from_bytes (gpg_data);
guint imported = 0;
if (!ostree_repo_remote_gpg_import (self->repo, remote_name, input_stream,
NULL, &imported, cancellable, error))
return FALSE;
/* XXX If we ever add internationalization, use ngettext() here. */
g_info ("Imported %u GPG key%s to remote \"%s\"",
imported, (imported == 1) ? "" : "s", remote_name);
}
goto out;
filter_path = g_key_file_get_value (new_config, group, "xa.filter", NULL);
if (filter_path && *filter_path && g_file_test (filter_path, G_FILE_TEST_EXISTS))
@@ -16114,10 +16116,28 @@ flatpak_dir_modify_remote (FlatpakDir *self,
/* If we e.g. changed url or gpg config the cached summary may be invalid */
if (!flatpak_dir_remote_clear_cached_summary (self, remote_name, cancellable, error))
return FALSE;
goto out;
if (!flatpak_dir_mark_changed (self, error))
return FALSE;
if (gpg_data != NULL)
{
g_autoptr(GInputStream) input_stream = g_memory_input_stream_new_from_bytes (gpg_data);
guint imported = 0;
if (!ostree_repo_remote_gpg_import (self->repo, remote_name, input_stream,
NULL, &imported, cancellable, error))
goto out;
/* XXX If we ever add internationalization, use ngettext() here. */
g_info ("Imported %u GPG key%s to remote \"%s\"",
imported, (imported == 1) ? "" : "s", remote_name);
}
{
g_autoptr(GError) local_error = NULL;
if (!flatpak_dir_mark_changed (self, &local_error))
g_warning ("Failed to mark dir as changed: %s", local_error->message);
}
if (has_remote)
flatpak_dir_log (self, "modify remote", remote_name, NULL, NULL, NULL, url,
@@ -16126,7 +16146,16 @@ flatpak_dir_modify_remote (FlatpakDir *self,
flatpak_dir_log (self, "add remote", remote_name, NULL, NULL, NULL, url,
"Added remote %s to %s", remote_name, url);
return TRUE;
res = TRUE;
out:
if (!res)
{
/* Roll back the changes. Ideally they would be atomic, because if the
* program terminates before we roll back, we end up in a broken state */
ostree_repo_write_config (self->repo, old_config, NULL);
ostree_repo_reload_config (self->repo, NULL, NULL);
}
return res;
}
gboolean

42
tests/test-repo.sh
View File

@@ -24,7 +24,7 @@ set -euo pipefail
skip_without_bwrap
skip_revokefs_without_fuse
echo "1..45"
echo "1..47"
#Regular repo
setup_repo
@@ -134,6 +134,46 @@ if ${FLATPAK} ${U} install -y test-repo org.test.Platform &> install-error-log;
fi
ok "failed to install again from different remote"
port=$(cat httpd-port)
echo "bad key" > badkey
gpg2_repo_url=$(ostree config --repo=$FL_DIR/repo get --group 'remote "test-gpg2-repo"' url)
gpg2_repo_key=$(cat "$FL_DIR/repo/test-gpg2-repo.trustedkeys.gpg")
${FLATPAK} ${U} remote-modify --gpg-import=badkey test-gpg2-repo >&2 || true
gpg2_repo_url2=$(ostree config --repo=$FL_DIR/repo get --group 'remote "test-gpg2-repo"' url)
gpg2_repo_key2=$(cat "$FL_DIR/repo/test-gpg2-repo.trustedkeys.gpg")
if [[ "${gpg2_repo_url}" != "${gpg2_repo_url2}" || "${gpg2_repo_key}" != "${gpg2_repo_key2}" ]]; then
assert_not_reached "remote-modify failed but remote was modified"
fi
${FLATPAK} ${U} remote-add --gpg-import=badkey test-broken-repo "http://127.0.0.1:${port}/test-broken-repo" >&2 && false
ostree config --repo=$FL_DIR/repo get --group 'remote "test-broken-repo"' url > /dev/null 2>&1 && \
assert_not_reached "Bug #6449: Remote added with broken GPG key"
[[ -f "$FL_DIR/repo/test-broken-repo.trustedkeys.gpg" ]] && false
ok "fail with broken repo"
cat << EOF > test-broken-repo.flatpakrepo
[Flatpak Repo]
Version=1
Url=http://no.127.0.0.1:$(cat httpd-port)/test-broken/
Title=The Remote Title
GPGKey=AAA${FL_GPG_BASE64}AAA
EOF
mkdir -p $FLATPAK_CONFIG_DIR/remotes.d
cp test-broken-repo.flatpakrepo $FLATPAK_CONFIG_DIR/remotes.d/
${FLATPAK} ${U} remote-add --from test-broken-repo >&2 && false
ostree config --repo=$FL_DIR/repo get --group 'remote "test-broken-repo"' url > /dev/null 2>&1 && \
assert_not_reached "Bug #6449: Remote added with broken GPG key"
[[ -f "$FL_DIR/repo/test-broken-repo.trustedkeys.gpg" ]] && false
rm -rf $FLATPAK_CONFIG_DIR/remotes.d/
ok "fail with statically configured broken repo"
${FLATPAK} ${U} install -y --reinstall test-repo org.test.Platform >&2
ok "re-install"