fix(rattler_cache): detect read-only filesystems in PackageCacheLayer

[olantwin]

Description

While testing @chrisburr's PR for rattler-fs (#2566) I noticed that is_readonly() only inspected the permission bits of the layer directory, so a cache layer on a filesystem that is mounted read-only (CVMFS, squashfs, a read-only bind mount) with mode-0755 directories was classified as writable. The cache then attempted to create lock files in the layer and failed with EROFS instead of treating it as a read-only layer.

As a work-around/solution, this PR also checks the ST_RDONLY flag of the containing filesystem via statvfs(3) on Unix. This unfortunately introduces a dependency on libc. Maybe there's a better way?

Repro: PackageCache::new_layered with a layer on a read-only bind mount (mount --bind dir mnt && mount -o remount,ro,bind mnt); split_layers() reports it as writable and acquire_global_lock() / get_or_fetch() fail with:
failed to interact with the package cache layer.

How Has This Been Tested?

Tested with #2566 on in an ALMA9 virtual machine with "mock" CMVFS mount, and on an AMLA9 cluster with a CVMFS mount (CERN's lxplus). No access to any suitable macOS or Windows machines to test.

AI Disclosure

• This PR contains AI-generated content.
  • I have tested any AI-generated content in my PR.
  • I take responsibility for any AI-generated content in my PR.

Tools: {e.g., Claude, Codex, GitHub Copilot, ChatGPT, etc.}

Claude (Fable 5) helped me understand why the cache was mounted writable, where the relevant code is and proposed possible solutions. I selected, understood and tested the solution.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added sufficient tests to cover my changes.

COMMENT: Not sure how to best test this, as adding the right filesystems to the CI will add a lot of complexity to the tests...

[baszalmstra]

The inline #[cfg(unix)] block with nested if let / unsafe makes is_readonly hard to follow. We already have this exact pattern as a free function in flock.rs (is_on_nfs_mount). Can we do the same here?

pub fn is_readonly(&self) -> bool {
    self.path.metadata().is_ok_and(|m| m.permissions().readonly())
        || is_mounted_readonly(&self.path)
}

#[cfg(unix)]
fn is_mounted_readonly(path: &Path) -> bool { /* statvfs check */ }

#[cfg(not(unix))]
fn is_mounted_readonly(_path: &Path) -> bool { false }

rustix is already in our dependency tree via tempfile, and rustix::fs::statvfs(path)?.f_flag.contains(StatVfsMountFlags::RDONLY) would drop the unsafe block and the CString dance entirely.

[olantwin]

Thanks for the prompt review!

The inline #[cfg(unix)] block with nested if let / unsafe makes is_readonly hard to follow. We already have this exact pattern as a free function in flock.rs (is_on_nfs_mount). Can we do the same here?

pub fn is_readonly(&self) -> bool {
    self.path.metadata().is_ok_and(|m| m.permissions().readonly())
        || is_mounted_readonly(&self.path)
}

#[cfg(unix)]
fn is_mounted_readonly(path: &Path) -> bool { /* statvfs check */ }

#[cfg(not(unix))]
fn is_mounted_readonly(_path: &Path) -> bool { false }

I agree that your proposed arrangement is much easier to follow! Thanks for the pointer to flock.rs, the pattern is pretty clear there.

rustix is already in our dependency tree via tempfile, and rustix::fs::statvfs(path)?.f_flag.contains(StatVfsMountFlags::RDONLY) would drop the unsafe block and the CString dance entirely.

As someone relatively new to rust, I had no idea about rustix! That's much nicer than using the C interfaces.

I'll update the PR ASAP. For review, do you prefer that I just amend the previous commit or a squash at the end?

[baszalmstra]

I'll update the PR ASAP. For review, do you prefer that I just amend the previous commit or a squash at the end?

The PR is squashed when merged. For reviewing its easier to just add new commits. But feel free to do whatever.

[olantwin]

@baszalmstra May I ask you for a review, please?

[baszalmstra]

You still have CI issues.

[olantwin]

You still have CI issues.

Apologies, since those where for the python bindings, I wasn't sure they were related. Investigating now.

[baszalmstra]

Maybe just a rebase/merge is enough

[olantwin]

It seems to be a lock-file issue due to adding rustix to the crate. Hopefully regenerating them now fixed things.

[olantwin]

The semver check seems to be exhausting the disk space on the runner.

[olantwin]

@baszalmstra Thanks a lot for the feedback and guidance!