diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5154c3..0d30533 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,14 +131,17 @@ jobs: - name: Run unit tests run: cargo test --workspace - - name: Run MinIO integration tests + - name: Install FUSE for mount integration tests + run: sudo apt-get update && sudo apt-get install -y fuse3 + + - name: Run JuiceFS integration tests env: SMOLFS_RUN_INTEGRATION: "1" SMOLFS_TEST_REDIS_METADATA_URL: redis://127.0.0.1:6379/1 SMOLFS_TEST_S3_BUCKET: http://127.0.0.1:9000/smolfs-ci ACCESS_KEY: minioadmin SECRET_KEY: minioadmin - run: cargo test -p smolfs-juicefs --test minio_integration -- --nocapture + run: cargo test -p smolfs-juicefs --test minio_integration --test readonly_mount -- --nocapture - name: Build Python wheel run: uvx maturin==${{ env.MATURIN_VERSION }} build --manifest-path bindings/python/Cargo.toml --interpreter python --out dist diff --git a/README.md b/README.md index 8d1a5b5..cbcb31e 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,12 @@ Useful options: completes. - `--foreground` runs the mount process in the foreground instead of starting it in the background. +- `--readonly` (alias `--read-only`) mounts the volume read-only. This is useful + for data the current agent should not modify, such as model weights, + training/evaluation data, or reference documents produced by another process. + Writes to a read-only mount are rejected by the filesystem with + `Read-only file system` (EROFS), and `smolfs flush NAME` returns an error + because there is nothing to flush. ### Flush, Inspect, and Unmount @@ -258,9 +264,9 @@ cat ./workspace/hello.txt | `smolfs doctor` | Checks SmolFS storage, local mount support, and configuration. | | `smolfs init NAME --dev` | Creates a local development volume. | | `smolfs init NAME --metadata URL --storage TYPE --bucket BUCKET` | Creates a cloud volume with explicit metadata and object storage. | -| `smolfs mount NAME PATH` | Mounts a volume at a local directory. | -| `smolfs flush NAME` | Probes the mounted volume and syncs a small file through it. | -| `smolfs status [NAME]` | Shows known volumes and current mountpoints. | +| `smolfs mount NAME PATH` | Mounts a volume at a local directory. Add `--readonly` to mount read-only. | +| `smolfs flush NAME` | Probes the mounted volume and syncs a small file through it. Errors on a read-only mount. | +| `smolfs status [NAME]` | Shows known volumes, mountpoints, and mount mode (`ro`/`rw`). | | `smolfs unmount NAME` | Unmounts a mounted volume and asks SmolFS to flush. | | `smolfs umount NAME` | Alias for `smolfs unmount NAME`. | diff --git a/bindings/node/index.d.ts b/bindings/node/index.d.ts index 315f067..2cc5440 100644 --- a/bindings/node/index.d.ts +++ b/bindings/node/index.d.ts @@ -15,6 +15,7 @@ export interface MountVolumeOptions { name: string path: string checkStorage?: boolean + readOnly?: boolean } export interface UnmountOptions { force?: boolean @@ -26,6 +27,7 @@ export interface VolumeInfo { bucket: string dev: boolean mountpoint?: string + readOnly: boolean } export interface MountInfo { name: string diff --git a/bindings/node/src/lib.rs b/bindings/node/src/lib.rs index 6a99176..e5ebd2e 100644 --- a/bindings/node/src/lib.rs +++ b/bindings/node/src/lib.rs @@ -20,6 +20,7 @@ pub struct MountVolumeOptions { pub name: String, pub path: String, pub check_storage: Option, + pub read_only: Option, } #[napi(object)] @@ -35,6 +36,7 @@ pub struct VolumeInfo { pub bucket: String, pub dev: bool, pub mountpoint: Option, + pub read_only: bool, } #[napi(object)] @@ -172,6 +174,7 @@ impl From for MountVolume { path: PathBuf::from(value.path), foreground: false, check_storage: value.check_storage.unwrap_or(false), + readonly: value.read_only.unwrap_or(false), } } } @@ -185,6 +188,7 @@ impl From for VolumeInfo { bucket: value.bucket, dev: value.dev, mountpoint: value.mountpoint.map(|path| path.display().to_string()), + read_only: value.read_only, } } } diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs index 6c909d8..a891d3c 100644 --- a/bindings/python/src/lib.rs +++ b/bindings/python/src/lib.rs @@ -28,6 +28,8 @@ struct VolumeInfo { dev: bool, #[pyo3(get)] mountpoint: Option, + #[pyo3(get)] + read_only: bool, } #[pyclass] @@ -106,13 +108,14 @@ impl PySmolFs { .map_err(to_py_err) } - #[pyo3(signature = (name, path, *, foreground=false, check_storage=false))] + #[pyo3(signature = (name, path, *, foreground=false, check_storage=false, read_only=false))] fn mount( &self, name: String, path: String, foreground: bool, check_storage: bool, + read_only: bool, ) -> PyResult { self.inner .mount(MountVolume { @@ -120,6 +123,7 @@ impl PySmolFs { path: PathBuf::from(path), foreground, check_storage, + readonly: read_only, }) .map(MountInfo::from) .map_err(to_py_err) @@ -154,6 +158,7 @@ impl From for VolumeInfo { bucket: value.bucket, dev: value.dev, mountpoint: value.mountpoint.map(|path| path.display().to_string()), + read_only: value.read_only, } } } diff --git a/crates/smolfs-cli/src/main.rs b/crates/smolfs-cli/src/main.rs index f4c241b..e1fb777 100644 --- a/crates/smolfs-cli/src/main.rs +++ b/crates/smolfs-cli/src/main.rs @@ -60,6 +60,12 @@ enum Command { foreground: bool, #[arg(long, help = "Test object storage access before mounting")] check_storage: bool, + #[arg( + long, + visible_alias = "read-only", + help = "Mount the volume read-only; writes are rejected by the filesystem" + )] + readonly: bool, }, #[command(about = "Show configured SmolFS volumes")] Status { @@ -159,6 +165,7 @@ fn main() -> Result<()> { path, foreground, check_storage, + readonly, } => { let fs = SmolFs::from_env()?; let mount = fs.mount(MountVolume { @@ -166,11 +173,13 @@ fn main() -> Result<()> { path, foreground, check_storage, + readonly, })?; println!( - "Mounted volume {} at {}", + "Mounted volume {} at {}{}", mount.name, - mount.mountpoint.display() + mount.mountpoint.display(), + if readonly { " (read-only)" } else { "" } ); } Command::Status { name, json } => { @@ -187,12 +196,20 @@ fn main() -> Result<()> { .as_ref() .map(|path| path.display().to_string()) .unwrap_or_else(|| "-".into()); + let mode = if volume.mountpoint.is_none() { + "-" + } else if volume.read_only { + "ro" + } else { + "rw" + }; println!( - "{}\t{}\t{}\t{}", + "{}\t{}\t{}\t{}\t{}", volume.name, if volume.dev { "dev" } else { "cloud" }, volume.storage, - mountpoint + mountpoint, + mode ); } } diff --git a/crates/smolfs-core/src/error.rs b/crates/smolfs-core/src/error.rs index fe4c425..27dbb94 100644 --- a/crates/smolfs-core/src/error.rs +++ b/crates/smolfs-core/src/error.rs @@ -39,6 +39,9 @@ pub enum SmolFsError { #[error("volume {name:?} is not mounted")] VolumeNotMounted { name: String }, + #[error("volume {name:?} is mounted read-only")] + VolumeReadOnly { name: String }, + #[error("local mount support is unavailable: {detail}\nFix: {fix}")] MountSupportUnavailable { detail: String, fix: String }, diff --git a/crates/smolfs-core/src/models.rs b/crates/smolfs-core/src/models.rs index e95cd27..3c8420a 100644 --- a/crates/smolfs-core/src/models.rs +++ b/crates/smolfs-core/src/models.rs @@ -11,6 +11,8 @@ pub struct Volume { pub bucket: String, pub dev: bool, pub mountpoint: Option, + #[serde(default)] + pub read_only: bool, pub created_at: String, pub updated_at: String, } @@ -31,6 +33,7 @@ pub struct MountVolume { pub path: PathBuf, pub foreground: bool, pub check_storage: bool, + pub readonly: bool, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -47,6 +50,7 @@ pub struct VolumeInfo { pub bucket: String, pub dev: bool, pub mountpoint: Option, + pub read_only: bool, } impl From<&Volume> for VolumeInfo { @@ -58,6 +62,7 @@ impl From<&Volume> for VolumeInfo { bucket: volume.bucket.clone(), dev: volume.dev, mountpoint: volume.mountpoint.clone(), + read_only: volume.read_only, } } } @@ -89,3 +94,29 @@ pub struct MountSupportReport { pub struct StatusReport { pub volumes: Vec, } + +#[cfg(test)] +mod tests { + use super::{Volume, VolumeInfo}; + + fn volume(read_only: bool) -> Volume { + Volume { + name: "demo".into(), + backend: "juicefs".into(), + metadata_url: "sqlite3://demo.db".into(), + storage: "file".into(), + bucket: "/tmp/objects".into(), + dev: true, + mountpoint: None, + read_only, + created_at: "now".into(), + updated_at: "now".into(), + } + } + + #[test] + fn volume_info_carries_read_only() { + assert!(VolumeInfo::from(&volume(true)).read_only); + assert!(!VolumeInfo::from(&volume(false)).read_only); + } +} diff --git a/crates/smolfs-core/src/registry.rs b/crates/smolfs-core/src/registry.rs index 9e1c62e..bf447de 100644 --- a/crates/smolfs-core/src/registry.rs +++ b/crates/smolfs-core/src/registry.rs @@ -130,6 +130,7 @@ mod tests { bucket: "/tmp/objects".into(), dev: true, mountpoint: None, + read_only: false, created_at: "now".into(), updated_at: "now".into(), }; @@ -138,4 +139,28 @@ mod tests { assert_eq!(registry.read("demo").unwrap().name, "demo"); assert_eq!(registry.list().unwrap().len(), 1); } + + #[test] + fn reads_legacy_volume_without_read_only_field() { + let temp = tempfile::tempdir().unwrap(); + let home = SmolFsHome::new(temp.path()); + home.ensure_layout().unwrap(); + let path = home.volume_path("legacy"); + // A volume record written before the read_only field existed. + let legacy = r#" +name = "legacy" +backend = "juicefs" +metadata_url = "sqlite3://legacy.db" +storage = "file" +bucket = "/tmp/objects" +dev = true +created_at = "now" +updated_at = "now" +"#; + std::fs::write(&path, legacy).unwrap(); + + let registry = Registry::new(home); + let volume = registry.read("legacy").unwrap(); + assert!(!volume.read_only); + } } diff --git a/crates/smolfs-juicefs/src/juicefs.rs b/crates/smolfs-juicefs/src/juicefs.rs index b9912a4..319dfe3 100644 --- a/crates/smolfs-juicefs/src/juicefs.rs +++ b/crates/smolfs-juicefs/src/juicefs.rs @@ -46,6 +46,9 @@ impl JuiceFs { if opts.check_storage { spec = spec.arg("--check-storage"); } + if opts.readonly { + spec = spec.arg("--read-only"); + } spec } @@ -97,10 +100,22 @@ impl JuiceFs { #[cfg(test)] mod tests { - use smolfs_core::Volume; + use std::path::PathBuf; + + use smolfs_core::{MountVolume, Volume}; use super::JuiceFs; + fn mount_opts(readonly: bool) -> MountVolume { + MountVolume { + name: "demo".into(), + path: PathBuf::from("/tmp/demo"), + foreground: false, + check_storage: false, + readonly, + } + } + fn volume() -> Volume { Volume { name: "demo".into(), @@ -110,6 +125,7 @@ mod tests { bucket: "/tmp/objects".into(), dev: true, mountpoint: None, + read_only: false, created_at: "now".into(), updated_at: "now".into(), } @@ -123,4 +139,20 @@ mod tests { assert!(spec.args.contains(&"--storage".to_string())); assert!(spec.args.contains(&"--bucket".to_string())); } + + #[test] + fn mount_command_includes_read_only_when_requested() { + let juicefs = JuiceFs::new("/bin/juicefs"); + let log = PathBuf::from("/tmp/demo.log"); + let spec = juicefs.mount_spec(&volume(), &mount_opts(true), &log); + assert!(spec.args.contains(&"--read-only".to_string())); + } + + #[test] + fn mount_command_omits_read_only_by_default() { + let juicefs = JuiceFs::new("/bin/juicefs"); + let log = PathBuf::from("/tmp/demo.log"); + let spec = juicefs.mount_spec(&volume(), &mount_opts(false), &log); + assert!(!spec.args.contains(&"--read-only".to_string())); + } } diff --git a/crates/smolfs-juicefs/src/service.rs b/crates/smolfs-juicefs/src/service.rs index ec32fae..aae0c43 100644 --- a/crates/smolfs-juicefs/src/service.rs +++ b/crates/smolfs-juicefs/src/service.rs @@ -75,14 +75,20 @@ impl SmolFs { let mountpoint = absolute_path(&opts.path)?; let log_path = self.home.volume_log_path(&opts.name); + let readonly = opts.readonly; let command_opts = MountVolume { path: mountpoint.clone(), ..opts }; self.juicefs.mount(&volume, &command_opts, &log_path)?; - probe_mount(&mountpoint)?; + if readonly { + probe_mount_readonly(&mountpoint)?; + } else { + probe_mount(&mountpoint)?; + } volume.mountpoint = Some(mountpoint.clone()); + volume.read_only = readonly; volume.updated_at = now_rfc3339(); self.registry.write(&volume)?; @@ -95,6 +101,11 @@ impl SmolFs { pub fn flush(&self, name: &str) -> Result<()> { validate_volume_name(name)?; let volume = self.registry.read(name)?; + if volume.read_only { + return Err(SmolFsError::VolumeReadOnly { + name: name.to_string(), + }); + } let mountpoint = volume .mountpoint .ok_or_else(|| SmolFsError::VolumeNotMounted { @@ -118,6 +129,7 @@ impl SmolFs { })?; self.juicefs.unmount(&mountpoint, force)?; volume.mountpoint = None; + volume.read_only = false; volume.updated_at = now_rfc3339(); self.registry.write(&volume) } @@ -177,6 +189,7 @@ impl SmolFs { bucket, dev: opts.dev, mountpoint: None, + read_only: false, created_at: now.clone(), updated_at: now, }) @@ -223,6 +236,16 @@ fn probe_mount(path: &Path) -> Result<()> { }) } +fn probe_mount_readonly(path: &Path) -> Result<()> { + // A read-only mount rejects writes, so verify liveness with a read instead. + fs::read_dir(path) + .map(|_| ()) + .map_err(|source| SmolFsError::IoAt { + path: path.to_path_buf(), + source, + }) +} + fn sync_probe(path: &Path) -> Result<()> { let probe = path.join(".smolfs-flush"); let file = fs::File::create(&probe).map_err(|source| SmolFsError::IoAt { @@ -240,3 +263,54 @@ fn sync_probe(path: &Path) -> Result<()> { } Ok(()) } + +#[cfg(all(test, unix))] +mod tests { + use std::os::unix::fs::PermissionsExt; + + use smolfs_core::{Registry, SmolFsError, SmolFsHome, Volume}; + + use super::SmolFs; + + fn stub_backend(home: &SmolFsHome) { + home.ensure_layout().unwrap(); + let bin = home.managed_storage_backend_bin(); + std::fs::write(&bin, b"#!/bin/sh\nexit 0\n").unwrap(); + let mut perms = std::fs::metadata(&bin).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&bin, perms).unwrap(); + } + + fn mounted_volume(name: &str, read_only: bool) -> Volume { + Volume { + name: name.into(), + backend: "juicefs".into(), + metadata_url: "sqlite3://demo.db".into(), + storage: "file".into(), + bucket: "/tmp/objects".into(), + dev: true, + mountpoint: Some("/tmp/does-not-need-to-exist".into()), + read_only, + created_at: "now".into(), + updated_at: "now".into(), + } + } + + #[test] + fn flush_errors_on_read_only_volume() { + let temp = tempfile::tempdir().unwrap(); + let home = SmolFsHome::new(temp.path()); + stub_backend(&home); + Registry::new(home.clone()) + .write(&mounted_volume("ro", true)) + .unwrap(); + + let fs = SmolFs::new(home).unwrap(); + // The read-only guard fires before any mount I/O, so no real mount is needed. + let err = fs.flush("ro").unwrap_err(); + assert!( + matches!(err, SmolFsError::VolumeReadOnly { name } if name == "ro"), + "expected VolumeReadOnly error" + ); + } +} diff --git a/crates/smolfs-juicefs/tests/readonly_mount.rs b/crates/smolfs-juicefs/tests/readonly_mount.rs new file mode 100644 index 0000000..507f91c --- /dev/null +++ b/crates/smolfs-juicefs/tests/readonly_mount.rs @@ -0,0 +1,152 @@ +use std::env; +use std::io::ErrorKind; +use std::path::Path; + +use smolfs_core::{Config, InitVolume, MountVolume, SmolFsError, SmolFsHome}; +use smolfs_juicefs::{SmolFs, install_managed_storage_backend}; + +struct MetadataBackend { + name: &'static str, + url: String, +} + +fn integration_enabled() -> bool { + env::var("SMOLFS_RUN_INTEGRATION").as_deref() == Ok("1") +} + +/// Exercises the full read-only mount lifecycle against a real JuiceFS backend. +/// +/// Per `AGENTS.md`, JuiceFS behavior must be covered against both the Redis and +/// SQLite metadata engines, so the lifecycle runs once per backend, both backed +/// by the same S3-compatible object store. Gated behind +/// `SMOLFS_RUN_INTEGRATION=1` because it needs the storage backend binary, the +/// metadata services, and local mount (FUSE) support. +#[test] +fn read_only_mount_rejects_writes_and_flush() { + if !integration_enabled() { + eprintln!("skipping read-only mount integration test; set SMOLFS_RUN_INTEGRATION=1"); + return; + } + + let bucket = env::var("SMOLFS_TEST_S3_BUCKET") + .unwrap_or_else(|_| "http://127.0.0.1:9000/smolfs-ci".to_string()); + + let temp = tempfile::tempdir().expect("create temp SmolFS home"); + let home = SmolFsHome::new(temp.path()); + + if let Ok(bin) = env::var("SMOLFS_STORAGE_BACKEND_BIN") { + Config { + storage_backend_bin: Some(bin.into()), + } + .save(&home) + .expect("save storage backend path"); + } else { + install_managed_storage_backend(&home).expect("install managed storage backend"); + } + + let fs = SmolFs::new(home).expect("construct SmolFS service"); + + for metadata in metadata_backends(temp.path()) { + run_lifecycle(&fs, &metadata, &bucket, temp.path()); + } +} + +fn run_lifecycle(fs: &SmolFs, metadata: &MetadataBackend, bucket: &str, root: &Path) { + let name = format!("ro-ci-{}-{}", metadata.name, std::process::id()); + + fs.init(InitVolume { + name: name.clone(), + dev: false, + metadata_url: Some(metadata.url.clone()), + store_url: None, + storage: Some("s3".to_string()), + bucket: Some(bucket.to_string()), + }) + .unwrap_or_else(|err| panic!("init volume with {} metadata: {err}", metadata.name)); + + // --- Read-only mount --- + let ro_dir = root.join(format!("{}-ro", metadata.name)); + fs.mount(MountVolume { + name: name.clone(), + path: ro_dir.clone(), + foreground: false, + check_storage: false, + readonly: true, + }) + .unwrap_or_else(|err| panic!("mount read-only with {} metadata: {err}", metadata.name)); + + let status = fs.status(Some(&name)).expect("status after ro mount"); + assert!( + status.volumes[0].read_only, + "{}: status should report read_only=true", + metadata.name + ); + + // A write must be rejected by the filesystem (EROFS). + let write_err = std::fs::write(ro_dir.join("nope.txt"), b"blocked") + .expect_err("write to a read-only mount must fail"); + assert!( + write_err.kind() == ErrorKind::ReadOnlyFilesystem + || write_err.raw_os_error() == Some(EROFS), + "{}: expected a read-only filesystem error, got {write_err:?}", + metadata.name + ); + + // flush must error loudly on a read-only mount. + let flush_err = fs + .flush(&name) + .expect_err("flush on read-only mount must fail"); + assert!( + matches!(flush_err, SmolFsError::VolumeReadOnly { .. }), + "{}: expected VolumeReadOnly, got {flush_err:?}", + metadata.name + ); + + fs.unmount(&name, false).expect("unmount read-only"); + + // --- Read-write remount: writes and flush must work again --- + let rw_dir = root.join(format!("{}-rw", metadata.name)); + fs.mount(MountVolume { + name: name.clone(), + path: rw_dir.clone(), + foreground: false, + check_storage: false, + readonly: false, + }) + .unwrap_or_else(|err| panic!("remount read-write with {} metadata: {err}", metadata.name)); + + let status = fs.status(Some(&name)).expect("status after rw mount"); + assert!( + !status.volumes[0].read_only, + "{}: status should report read_only=false after rw remount", + metadata.name + ); + + std::fs::write(rw_dir.join("ok.txt"), b"allowed").expect("write to a read-write mount"); + fs.flush(&name).expect("flush a read-write mount"); + + fs.unmount(&name, false).expect("unmount read-write"); +} + +fn metadata_backends(root: &Path) -> Vec { + let redis_url = env::var("SMOLFS_TEST_REDIS_METADATA_URL") + .or_else(|_| env::var("SMOLFS_TEST_METADATA_URL")) + .unwrap_or_else(|_| "redis://127.0.0.1:6379/1".to_string()); + let sqlite_path = root.join("metadata.sqlite3"); + let sqlite_url = env::var("SMOLFS_TEST_SQLITE_METADATA_URL") + .unwrap_or_else(|_| format!("sqlite3://{}", sqlite_path.to_string_lossy())); + + vec![ + MetadataBackend { + name: "redis", + url: redis_url, + }, + MetadataBackend { + name: "sqlite", + url: sqlite_url, + }, + ] +} + +/// EROFS ("Read-only file system") is errno 30 on both Linux and macOS. +const EROFS: i32 = 30;