Skip to content

Conversation

@kipz
Copy link
Contributor

@kipz kipz commented Nov 5, 2025

Summary

Fixes unsafe local mode to properly load root metadata rotations from disk when operating offline.

Previously, unsafe local mode would only use the initial trusted root and wouldn't recognize rotated roots that were downloaded while online.

This change adds support for loading rotated roots from disk in unsafe local mode, allowing it to work correctly even after root rotations have occurred.

kipz added 2 commits November 5, 2025 14:46
This test verifies that unsafe local mode can correctly load
root metadata rotations from disk when operating offline.

The test:
- Runs initial refresh to establish metadata
- Rotates root metadata twice (v1 -> v2 -> v3)
- Downloads rotated roots in online mode
- Creates unsafe updater with original root (v1)
- Verifies it loads all rotated roots from disk to v3

This test currently fails because unsafe local mode doesn't
load rotated roots from disk.

Signed-off-by: James Carnegie <[email protected]>
Previously, unsafe local mode would only use the initial trusted
root and wouldn't load any rotated roots from disk. This meant
that if roots were rotated while online, then switching to unsafe
local mode would fail to recognize the newer root versions.

This fix adds:
- loadRootFromDisk(): New method that loads rotated root metadata
  from local disk, similar to how online mode loads from remote
- Updated unsafeLocalRefresh(): Now calls loadRootFromDisk() to
  load any rotated roots before loading other metadata
- Updated loadRoot(): Persists versioned root files (N.root.json)
  to disk for offline use by unsafe local mode

This allows unsafe local mode to properly handle root rotations
when operating offline, as long as the rotated roots were
previously downloaded in online mode.

Signed-off-by: James Carnegie <[email protected]>
@kipz kipz force-pushed the kipz/unsafe-local branch from 3a4cdb7 to 96445cd Compare November 5, 2025 14:46
@kommendorkapten
Copy link
Member

kommendorkapten commented Nov 7, 2025

Thanks for your PR @kipz

For the current implementation to break, there must have been a root rotation since the time since the last Refresh(), but existing (in the local cache) timestamp still valid.
If this is failing, it would be better to perform a proper Refresh() as there there may be a risk that more delegates has changed, the client will be at risk to use stale data for other roles (consider that there is a new targets update, that removes or edits an existing file).

I would strongly consider to look into how Sigstore is dealing with unsafe cached metadata: https://github.com/sigstore/sigstore-go/blob/b48a7c15af434de768f6db5d729a9aadafab5060/pkg/tuf/client.go#L38

In short, if the unsafe local cache version fails, it automatically fallbacks to a full refresh to avoid using any stale (potentially revoked) data. This client also implements a forced Refresh method, which mitigates the need for #701

@kipz
Copy link
Contributor Author

kipz commented Nov 18, 2025

Thanks @kommendorkapten . I've been playing with another implementation: kipz@f375082 - but I still think it's pretty confusing for the user....

@kommendorkapten
Copy link
Member

We must also remember that using cached metadata from disk poses a risk, and is in violation with the TUF framework, so having more complexity in the client that possibly can act as a foot-gun is some thing I want to avoid.

What is the scenario you are looking into when this is needed?

@kipz
Copy link
Contributor Author

kipz commented Nov 19, 2025

@kommendorkapten yeah, I appreciate that we would need to treat the disk as if it were the remote repo to get the same guarantees. Thinking about this some more, what I'm really after is some speed up for client (updater) creation. Right now, Refresh() always fetches timestamps even if the timestamp is valid - doing this lazily wouldn't break spec I think? Similarly with root rotations - perhaps these could be done later too (or in the background?). Overall though, the tension here is that the updater only fetches root rotations and fresh timestamp metadata at startup, so is really only useful for ephemeral use (needs a forced refresh as you mention), but for ephemeral use cases, we are doing a lot of work in the hot path. 🤔 Will close or now though - you're right about the sigstore approach!

@kommendorkapten
Copy link
Member

Right now, Refresh() always fetches timestamps even if the timestamp is valid - doing this lazily wouldn't break spec I think?

Actually it would. The timestamp should be downloaded and verified secondly after the root. This is important as the timestamp signature specifies the snapshot version, and the snapshot specifies the targets version. So even if the timestamp on disk is valid, the targets role (and so all targets) could have been updated.

but for ephemeral use cases, we are doing a lot of work in the hot path.

Could this be refactored somehow? I don't know about how your architecture, but if there is a parent/child relation, it may work to refactor so the parent is the TUF client, pulls down and verifies the targets and then hands them over to the child? This way you can have a periodic refresh in the parent, and keep the TUF interactions out of the hot loop? Feel free to hook me up on slack or so for further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants