Skip to content

Commit fe47dfc

Browse files
committed
feat: enable Refresh() to be called multiple times
Allow Refresh() to be called multiple times during the lifetime of an Updater by removing workflow guards that prevented re-updating metadata after dependent metadata was loaded. The Update* methods in TrustedMetadata already handle atomic updates, version checking, and rollback protection. The guards were only needed to enforce workflow order within a single refresh cycle, but prevented multiple refresh cycles. This enables long-running processes to periodically refresh metadata without creating new Updater instances. Updates are atomic and thread-safe - metadata is never cleared, only replaced after validation. Fixes #593
1 parent c0e325d commit fe47dfc

File tree

3 files changed

+29
-26
lines changed

3 files changed

+29
-26
lines changed

metadata/trustedmetadata/trustedmetadata.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ func New(rootData []byte) (*TrustedMetadata, error) {
5757
func (trusted *TrustedMetadata) UpdateRoot(rootData []byte) (*metadata.Metadata[metadata.RootType], error) {
5858
log := metadata.GetLogger()
5959

60-
if trusted.Timestamp != nil {
61-
return nil, &metadata.ErrRuntime{Msg: "cannot update root after timestamp"}
62-
}
6360
log.Info("Updating root")
6461
// generate root metadata
6562
newRoot, err := metadata.Root().FromBytes(rootData)
@@ -99,9 +96,6 @@ func (trusted *TrustedMetadata) UpdateRoot(rootData []byte) (*metadata.Metadata[
9996
func (trusted *TrustedMetadata) UpdateTimestamp(timestampData []byte) (*metadata.Metadata[metadata.TimestampType], error) {
10097
log := metadata.GetLogger()
10198

102-
if trusted.Snapshot != nil {
103-
return nil, &metadata.ErrRuntime{Msg: "cannot update timestamp after snapshot"}
104-
}
10599
// client workflow 5.3.10: Make sure final root is not expired.
106100
if trusted.Root.Signed.IsExpired(trusted.RefTime) {
107101
// no need to check for 5.3.11 (fast forward attack recovery):
@@ -178,9 +172,6 @@ func (trusted *TrustedMetadata) UpdateSnapshot(snapshotData []byte, isTrusted bo
178172
if trusted.Timestamp == nil {
179173
return nil, &metadata.ErrRuntime{Msg: "cannot update snapshot before timestamp"}
180174
}
181-
if trusted.Targets[metadata.TARGETS] != nil {
182-
return nil, &metadata.ErrRuntime{Msg: "cannot update snapshot after targets"}
183-
}
184175
log.Info("Updating snapshot")
185176

186177
// snapshot cannot be loaded if final timestamp is expired

metadata/trustedmetadata/trustedmetadata_test.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,9 +227,10 @@ func TestOutOfOrderOps(t *testing.T) {
227227
_, err = trustedSet.UpdateTimestamp(allRoles[metadata.TIMESTAMP])
228228
assert.NoError(t, err)
229229

230-
// Update root after timestamp
230+
// Update root after timestamp is now allowed (for multiple Refresh() calls)
231+
// but will fail due to version number check (expects v2, has v1)
231232
_, err = trustedSet.UpdateRoot(allRoles[metadata.ROOT])
232-
assert.ErrorIs(t, err, &metadata.ErrRuntime{Msg: "cannot update root after timestamp"})
233+
assert.ErrorIs(t, err, &metadata.ErrBadVersionNumber{Msg: "bad version number, expected 2, got 1"})
233234

234235
// Update targets before snapshot
235236
_, err = trustedSet.UpdateTargets(allRoles[metadata.TARGETS])
@@ -238,9 +239,10 @@ func TestOutOfOrderOps(t *testing.T) {
238239
_, err = trustedSet.UpdateSnapshot(allRoles[metadata.SNAPSHOT], false)
239240
assert.NoError(t, err)
240241

241-
// Update timestamp after snapshot
242+
// Update timestamp after snapshot is now allowed (for multiple Refresh() calls)
243+
// and will succeed since versions are equal
242244
_, err = trustedSet.UpdateTimestamp(allRoles[metadata.TIMESTAMP])
243-
assert.ErrorIs(t, err, &metadata.ErrRuntime{Msg: "cannot update timestamp after snapshot"})
245+
assert.ErrorIs(t, err, &metadata.ErrEqualVersionNumber{Msg: "new timestamp version 1 equals the old one 1"})
244246

245247
// Update delegated targets before targets
246248
_, err = trustedSet.UpdateDelegatedTargets(allRoles["role1"], "role1", metadata.TARGETS)
@@ -249,9 +251,10 @@ func TestOutOfOrderOps(t *testing.T) {
249251
_, err = trustedSet.UpdateTargets(allRoles[metadata.TARGETS])
250252
assert.NoError(t, err)
251253

252-
// Update snapshot after sucessful targets update
254+
// Update snapshot after targets is now allowed (for multiple Refresh() calls)
255+
// and will succeed since versions are equal
253256
_, err = trustedSet.UpdateSnapshot(allRoles[metadata.SNAPSHOT], false)
254-
assert.ErrorIs(t, err, &metadata.ErrRuntime{Msg: "cannot update snapshot after targets"})
257+
assert.NoError(t, err)
255258

256259
_, err = trustedSet.UpdateDelegatedTargets(allRoles["role1"], "role1", metadata.TARGETS)
257260
assert.NoError(t, err)

metadata/updater/updater.go

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ func New(config *config.UpdaterConfig) (*Updater, error) {
102102
// Downloads, verifies, and loads metadata for the top-level roles in the
103103
// specified order (root -> timestamp -> snapshot -> targets) implementing
104104
// all the checks required in the TUF client workflow.
105-
// A Refresh() can be done only once during the lifetime of an Updater.
105+
// Refresh() can be called multiple times during the lifetime of an Updater
106+
// to ensure that the metadata is up-to-date. Each call will reload the
107+
// timestamp, snapshot, and targets metadata while preserving the root metadata.
106108
// If Refresh() has not been explicitly called before the first
107109
// GetTargetInfo() call, it will be done implicitly at that time.
108110
// The metadata for delegated roles is not updated by Refresh():
@@ -135,6 +137,9 @@ func (update *Updater) onlineRefresh() error {
135137
if err != nil {
136138
return err
137139
}
140+
// Remove the targets entry to allow re-loading during multiple Refresh() calls
141+
// while still preventing redundant loads during GetTargetInfo()
142+
delete(update.trusted.Targets, metadata.TARGETS)
138143
_, err = update.loadTargets(metadata.TARGETS, metadata.ROOT)
139144
if err != nil {
140145
return err
@@ -313,12 +318,16 @@ func (update *Updater) loadTimestamp() error {
313318
if errors.Is(err, &metadata.ErrRepository{}) {
314319
// local timestamp is not valid, proceed downloading from remote; note that this error type includes several other subset errors
315320
log.Info("Local timestamp is not valid")
321+
} else if errors.Is(err, &metadata.ErrEqualVersionNumber{}) {
322+
// local timestamp version equals current trusted version, proceed to check remote for updates
323+
log.Info("Local timestamp version equals trusted version")
316324
} else {
317325
// another error
318326
return err
319327
}
328+
} else {
329+
log.Info("Local timestamp is valid")
320330
}
321-
log.Info("Local timestamp is valid")
322331
// all okay, local timestamp exists and it is valid, nevertheless proceed with downloading from remote
323332
}
324333
// load from remote (whether local load succeeded or not)
@@ -368,12 +377,12 @@ func (update *Updater) loadSnapshot() error {
368377
}
369378
} else {
370379
// this means snapshot verification/loading succeeded
371-
log.Info("Local snapshot is valid: not downloading new one")
372-
return nil
380+
log.Info("Local snapshot is valid")
381+
// Continue to check remote for potential updates
373382
}
374383
}
375-
// local snapshot does not exist or is invalid, update from remote
376-
log.Info("Failed to load local snapshot")
384+
// check remote for updates (whether local load succeeded or not)
385+
log.Info("Checking remote for snapshot updates")
377386
if update.trusted.Timestamp == nil {
378387
return fmt.Errorf("trusted timestamp not set")
379388
}
@@ -422,7 +431,7 @@ func (update *Updater) loadTargets(roleName, parentName string) (*metadata.Metad
422431
log.Info("Local role does not exist", "role", roleName)
423432
} else {
424433
// successfully read a local targets metadata, so let's try to verify and load it to the trusted metadata set
425-
delegatedTargets, err := update.trusted.UpdateDelegatedTargets(data, roleName, parentName)
434+
_, err := update.trusted.UpdateDelegatedTargets(data, roleName, parentName)
426435
if err != nil {
427436
// this means targets verification/loading failed
428437
if errors.Is(err, &metadata.ErrRepository{}) {
@@ -434,12 +443,12 @@ func (update *Updater) loadTargets(roleName, parentName string) (*metadata.Metad
434443
}
435444
} else {
436445
// this means targets verification/loading succeeded
437-
log.Info("Local role is valid: not downloading new one", "role", roleName)
438-
return delegatedTargets, nil
446+
log.Info("Local role is valid", "role", roleName)
447+
// Continue to check remote for potential updates
439448
}
440449
}
441-
// local "roleName" does not exist or is invalid, update from remote
442-
log.Info("Failed to load local role", "role", roleName)
450+
// check remote for updates (whether local load succeeded or not)
451+
log.Info("Checking remote for role updates", "role", roleName)
443452
if update.trusted.Snapshot == nil {
444453
return nil, fmt.Errorf("trusted snapshot not set")
445454
}

0 commit comments

Comments
 (0)