Skip to content

Commit d9a8f9d

Browse files
committed
Transaction: Fail the resolve if xa.metadata invalid or missing
If we fail to parse xa.metadata from the summary cache or the commit xa.metadata we fail the resolve. If xa.metadata is missing in the commit we fail the resolve (it is always set in the summary cache, because summary update converts missing xa.metadata to "", so we either get that, or cache miss which leads to resolving from the commit. This means that op->resolved_metadata is always set during install and updates, which means we will show the app permissions. The transaction will also always make sure that this data actually matches what gets deployed. Before this change an invalid metadata in the summary cache could lead to a NULL resolved_metadata, which means we wouldn't print the app permissions, yet we would still deploy some metadata file that could have permissions. (NOTE: It would fail to deploy unless the xa.metadata in the commit matched the metadata file, but in this corner case we would't compare the summary and commit metadata, so they may differ.)
1 parent ba818f5 commit d9a8f9d

File tree

1 file changed

+55
-29
lines changed

1 file changed

+55
-29
lines changed

Diff for: common/flatpak-transaction.c

+55-29
Original file line numberDiff line numberDiff line change
@@ -2970,12 +2970,13 @@ emit_eol_and_maybe_skip (FlatpakTransaction *self,
29702970
g_signal_emit (self, signals[END_OF_LIFED_WITH_REBASE], 0, op->remote, flatpak_decomposed_get_ref (op->ref), op->eol, op->eol_rebase, previous_ids, &op->skip);
29712971
}
29722972

2973-
static void
2973+
static gboolean
29742974
mark_op_resolved (FlatpakTransactionOperation *op,
29752975
const char *commit,
29762976
GFile *sideload_path,
29772977
GBytes *metadata,
2978-
GBytes *old_metadata)
2978+
GBytes *old_metadata,
2979+
GError **error)
29792980
{
29802981
g_debug ("marking op %s:%s resolved to %s", kind_to_str (op->kind), flatpak_decomposed_get_ref (op->ref), commit ? commit : "-");
29812982

@@ -2997,13 +2998,12 @@ mark_op_resolved (FlatpakTransactionOperation *op,
29972998
if (metadata)
29982999
{
29993000
g_autoptr(GKeyFile) metakey = g_key_file_new ();
3000-
if (g_key_file_load_from_bytes (metakey, metadata, G_KEY_FILE_NONE, NULL))
3001-
{
3002-
op->resolved_metadata = g_bytes_ref (metadata);
3003-
op->resolved_metakey = g_steal_pointer (&metakey);
3004-
}
3005-
else
3006-
g_message ("Warning: Failed to parse metadata for %s\n", flatpak_decomposed_get_ref (op->ref));
3001+
if (!g_key_file_load_from_bytes (metakey, metadata, G_KEY_FILE_NONE, NULL))
3002+
return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA,
3003+
"Metadata for %s is invalid", flatpak_decomposed_get_ref (op->ref));
3004+
3005+
op->resolved_metadata = g_bytes_ref (metadata);
3006+
op->resolved_metakey = g_steal_pointer (&metakey);
30073007
}
30083008
if (old_metadata)
30093009
{
@@ -3014,31 +3014,40 @@ mark_op_resolved (FlatpakTransactionOperation *op,
30143014
op->resolved_old_metakey = g_steal_pointer (&metakey);
30153015
}
30163016
else
3017-
g_message ("Warning: Failed to parse old metadata for %s\n", flatpak_decomposed_get_ref (op->ref));
3017+
{
3018+
/* This shouldn't happen, but a NULL old metadata is safe (all permisssions are considered new) */
3019+
g_message ("Warning: Failed to parse old metadata for %s\n", flatpak_decomposed_get_ref (op->ref));
3020+
}
30183021
}
3022+
3023+
return TRUE;
30193024
}
30203025

3021-
static void
3026+
static gboolean
30223027
resolve_op_end (FlatpakTransaction *self,
30233028
FlatpakTransactionOperation *op,
30243029
const char *checksum,
30253030
GFile *sideload_path,
3026-
GBytes *metadata_bytes)
3031+
GBytes *metadata_bytes,
3032+
GError **error)
30273033
{
30283034
g_autoptr(GBytes) old_metadata_bytes = NULL;
30293035

30303036
old_metadata_bytes = load_deployed_metadata (self, op->ref, NULL, NULL);
3031-
mark_op_resolved (op, checksum, sideload_path, metadata_bytes, old_metadata_bytes);
3037+
if (!mark_op_resolved (op, checksum, sideload_path, metadata_bytes, old_metadata_bytes, error))
3038+
return FALSE;
30323039
emit_eol_and_maybe_skip (self, op);
3040+
return TRUE;
30333041
}
30343042

30353043

3036-
static void
3044+
static gboolean
30373045
resolve_op_from_commit (FlatpakTransaction *self,
30383046
FlatpakTransactionOperation *op,
30393047
const char *checksum,
30403048
GFile *sideload_path,
3041-
GVariant *commit_data)
3049+
GVariant *commit_data,
3050+
GError **error)
30423051
{
30433052
g_autoptr(GBytes) metadata_bytes = NULL;
30443053
g_autoptr(GVariant) commit_metadata = NULL;
@@ -3049,9 +3058,11 @@ resolve_op_from_commit (FlatpakTransaction *self,
30493058
commit_metadata = g_variant_get_child_value (commit_data, 0);
30503059
g_variant_lookup (commit_metadata, "xa.metadata", "&s", &xa_metadata);
30513060
if (xa_metadata == NULL)
3052-
g_message ("Warning: No xa.metadata in local commit %s ref %s", checksum, flatpak_decomposed_get_ref (op->ref));
3053-
else
3054-
metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata));
3061+
return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA,
3062+
"No xa.metadata in local commit %s ref %s",
3063+
checksum, flatpak_decomposed_get_ref (op->ref));
3064+
3065+
metadata_bytes = g_bytes_new (xa_metadata, strlen (xa_metadata));
30553066

30563067
if (g_variant_lookup (commit_metadata, "xa.download-size", "t", &download_size))
30573068
op->download_size = GUINT64_FROM_BE (download_size);
@@ -3061,15 +3072,19 @@ resolve_op_from_commit (FlatpakTransaction *self,
30613072
g_variant_lookup (commit_metadata, OSTREE_COMMIT_META_KEY_ENDOFLIFE, "s", &op->eol);
30623073
g_variant_lookup (commit_metadata, OSTREE_COMMIT_META_KEY_ENDOFLIFE_REBASE, "s", &op->eol_rebase);
30633074

3064-
resolve_op_end (self, op, checksum, sideload_path, metadata_bytes);
3075+
return resolve_op_end (self, op, checksum, sideload_path, metadata_bytes, error);
30653076
}
30663077

3078+
/* NOTE: In case of non-available summary this returns FALSE with a
3079+
* NULL error, but for other error cases it will be set.
3080+
*/
30673081
static gboolean
30683082
try_resolve_op_from_metadata (FlatpakTransaction *self,
30693083
FlatpakTransactionOperation *op,
30703084
const char *checksum,
30713085
GFile *sideload_path,
3072-
FlatpakRemoteState *state)
3086+
FlatpakRemoteState *state,
3087+
GError **error)
30733088
{
30743089
g_autoptr(GBytes) metadata_bytes = NULL;
30753090
guint64 download_size = 0;
@@ -3109,8 +3124,7 @@ try_resolve_op_from_metadata (FlatpakTransaction *self,
31093124
op->token_type = GINT32_FROM_LE (var_metadata_lookup_int32 (sparse_cache, FLATPAK_SPARSE_CACHE_KEY_TOKEN_TYPE, op->token_type));
31103125
}
31113126

3112-
resolve_op_end (self, op, checksum, sideload_path, metadata_bytes);
3113-
return TRUE;
3127+
return resolve_op_end (self, op, checksum, sideload_path, metadata_bytes, error);
31143128
}
31153129

31163130
static gboolean
@@ -3153,7 +3167,8 @@ resolve_ops (FlatpakTransaction *self,
31533167
* checksum we got was the version already installed.
31543168
*/
31553169
g_assert (op->resolved_commit != NULL);
3156-
mark_op_resolved (op, op->resolved_commit, NULL, NULL, NULL);
3170+
if (!mark_op_resolved (op, op->resolved_commit, NULL, NULL, NULL, error))
3171+
return FALSE;
31573172
continue;
31583173
}
31593174

@@ -3167,14 +3182,16 @@ resolve_ops (FlatpakTransaction *self,
31673182
op->skip = TRUE;
31683183
continue;
31693184
}
3170-
mark_op_resolved (op, checksum, NULL, metadata_bytes, NULL);
3185+
if (!mark_op_resolved (op, checksum, NULL, metadata_bytes, NULL, error))
3186+
return FALSE;
31713187
continue;
31723188
}
31733189

31743190
if (op->kind == FLATPAK_TRANSACTION_OPERATION_INSTALL_BUNDLE)
31753191
{
31763192
g_assert (op->commit != NULL);
3177-
mark_op_resolved (op, op->commit, NULL, op->external_metadata, NULL);
3193+
if (!mark_op_resolved (op, op->commit, NULL, op->external_metadata, NULL, error))
3194+
return FALSE;
31783195
continue;
31793196
}
31803197

@@ -3205,7 +3222,8 @@ resolve_ops (FlatpakTransaction *self,
32053222
if (commit_data == NULL)
32063223
return FALSE;
32073224

3208-
resolve_op_from_commit (self, op, checksum, NULL, commit_data);
3225+
if (!resolve_op_from_commit (self, op, checksum, NULL, commit_data, error))
3226+
return FALSE;
32093227
}
32103228
else
32113229
{
@@ -3264,9 +3282,16 @@ resolve_ops (FlatpakTransaction *self,
32643282
}
32653283

32663284
/* First try to resolve via metadata (if remote is available and its metadata matches the commit version) */
3267-
if (!try_resolve_op_from_metadata (self, op, checksum, sideload_path, state))
3285+
if (!try_resolve_op_from_metadata (self, op, checksum, sideload_path, state, &local_error))
32683286
{
3269-
/* Else try to load the commit object.
3287+
if (local_error)
3288+
{
3289+
/* Actual error, not just missing from summary */
3290+
g_propagate_error (error, g_steal_pointer (&local_error));
3291+
return FALSE;
3292+
}
3293+
3294+
/* Missing from summary, try to load the commit object.
32703295
* Note, we don't have a token here, so this will not work for authenticated apps.
32713296
* We handle this by catching the 401 http status and retrying. */
32723297
g_autoptr(GVariant) commit_data = NULL;
@@ -3302,7 +3327,8 @@ resolve_ops (FlatpakTransaction *self,
33023327
return FALSE;
33033328
}
33043329

3305-
resolve_op_from_commit (self, op, checksum, sideload_path, commit_data);
3330+
if (!resolve_op_from_commit (self, op, checksum, sideload_path, commit_data, error))
3331+
return FALSE;
33063332
}
33073333
}
33083334
}

0 commit comments

Comments
 (0)