Read-only local stores should avoid creating `temproots` #992

New issue

Open

opened 2025-09-12 15:49:24 +00:00 by raito · 5 comments

raito commented

2025-09-12 15:49:24 +00:00

Owner

Even under read-only(read-only-local-store xp feature), the local store will attempt createDirs(tempRootsDir);, this won't work if the local store is read only and this directory doesn't exist.

To avoid this scenario, do not fail if that directory cannot be created.

Temproots cannot be used as:

    if (config().readOnly) {
      debug("Read-only store doesn't support creating lock files for temp roots, but nothing can be deleted anyways.");
      co_return result::success();
    }

Even under `read-only`(`read-only-local-store` xp feature), the local store will attempt ` createDirs(tempRootsDir);`, this won't work if the local store is read only and this directory doesn't exist. To avoid this scenario, do not fail if that directory cannot be created. Temproots cannot be used as: ``` if (config().readOnly) { debug("Read-only store doesn't support creating lock files for temp roots, but nothing can be deleted anyways."); co_return result::success(); } ```

raito added the

E/easy

Area/store

labels

2025-09-12 15:49:58 +00:00

pennae commented

2025-09-13 17:32:05 +00:00

Owner

note that this is necessarily unsound without a storage format update: a concurrent read-write store connection could delete existing store paths that are in use by read-only connections. the only way to make this sound is to force all connections to take a lock of some kind, such that no read-write connection can exist while read-only connections are active. since that reverses the usual lock order and can bring a system to a complete halt when used without great care we're not convinced that read-only local store connections are safe under any circumstances

note that this is necessarily unsound without a storage format update: a concurrent read-write store connection could delete existing store paths that are in use by read-only connections. the only way to make this sound is to force *all* connections to take a lock of some kind, such that no read-write connection can exist while read-only connections are active. since that reverses the usual lock order and can bring a system to a complete halt when used without great care we're not convinced that read-only local store connections are safe under *any* circumstances

jade commented

2025-09-13 22:12:14 +00:00

Owner

this should be safe provided that we do try to take the temp root no matter what and just tolerate failure in the case of read only connections (but permissions issues because you're regular user on a system with a daemon means it is unsound ...)

this should be safe provided that we *do* try to take the temp root no matter what and just tolerate failure in the case of read only connections (but permissions issues because you're regular user on a system with a daemon means it is unsound ...)

pennae commented

2025-09-13 22:21:31 +00:00

Owner

it really isn't. the readonlyness may be implied by a ro bind mount into a container, simply not setting a temproot for those can cause in-use paths to just disappear. the only safe use for ro stores is when rw stores are excluded or no gc can be run, both of which we can never guarantee without modifying the file system (or at least our db layout)

raito commented

2025-09-15 16:38:16 +00:00

Author

Owner

After some discussion, I believe it is safe in one special scenario: when the source is truly read-only, i.e. any process on the system will receive errors trying to write inside the local store. There's various guarantees level of this concept: a read-only block or fs (virtio bind RO, EROFS, etc.), a filesystem already mounted in ro with no possibility to remount `rw, etc.

It feels like more a dangerous-features than an experimental-features at this point, so I would be inclined to probably explore this sort of concept and extend the relevant code for this dangerous feature?

After some discussion, I believe it is safe in one special scenario: when the source is **truly** read-only, i.e. any process on the system will receive errors trying to write inside the local store. There's various guarantees level of this concept: a read-only block or fs (virtio bind RO, EROFS, etc.), a filesystem already mounted in `ro` with no possibility to remount `rw, etc. It feels like more a `dangerous-features` than an `experimental-features` at this point, so I would be inclined to probably explore this sort of concept and extend the relevant code for this *dangerous feature*?

jade commented

2025-09-15 18:28:08 +00:00

Owner

We could also loudly warn about it for now.