Expose chown functionality to FileSystem trait (#164)

Copilot · jaybosamiya-ms · web-flow · commit 6a534c5492ec · 2025-06-20T15:39:29.000-07:00
This PR adds `chown` functionality to the LiteBox filesystem API, following the same pattern as the existing `chmod` implementation. Fixes #163. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jaybosamiya-ms <171180729+jaybosamiya-ms@users.noreply.github.com> Co-authored-by: Jay Bosamiya (Microsoft) <jayb@microsoft.com>
diff --git a/litebox/src/fs/devices/stdio.rs b/litebox/src/fs/devices/stdio.rs
@@ -8,8 +8,8 @@ use crate::{
     fs::{
         FileStatus, FileType, Mode, OFlags, SeekWhence,
         errors::{
-            ChmodError, CloseError, FileStatusError, MkdirError, OpenError, PathError, ReadError,
-            RmdirError, SeekError, UnlinkError, WriteError,
+            ChmodError, ChownError, CloseError, FileStatusError, MkdirError, OpenError, PathError,
+            ReadError, RmdirError, SeekError, UnlinkError, WriteError,
         },
     },
     path::Arg,
@@ -136,6 +136,15 @@ impl<Platform: crate::platform::StdioProvider> super::super::FileSystem for File
         unimplemented!()
     }
 
+    fn chown(
+        &self,
+        path: impl Arg,
+        user: Option<u16>,
+        group: Option<u16>,
+    ) -> Result<(), ChownError> {
+        unimplemented!()
+    }
+
     fn unlink(&self, path: impl Arg) -> Result<(), UnlinkError> {
         unimplemented!()
     }
diff --git a/litebox/src/fs/errors.rs b/litebox/src/fs/errors.rs
@@ -72,6 +72,21 @@ pub enum ChmodError {
     PathError(#[from] PathError),
 }
 
+/// Possible errors from [`FileSystem::chown`]
+#[non_exhaustive]
+#[derive(Error, Debug)]
+pub enum ChownError {
+    #[error(
+        "the effective UID does not match the owner of the file, \
+         and the process is not privileged"
+    )]
+    NotTheOwner,
+    #[error("the named file resides on a read-only filesystem")]
+    ReadOnlyFileSystem,
+    #[error(transparent)]
+    PathError(#[from] PathError),
+}
+
 /// Possible errors from [`FileSystem::unlink`]
 #[non_exhaustive]
 #[derive(Error, Debug)]
diff --git a/litebox/src/fs/in_mem.rs b/litebox/src/fs/in_mem.rs
@@ -11,8 +11,8 @@ use crate::path::Arg;
 use crate::sync;
 
 use super::errors::{
-    ChmodError, CloseError, FileStatusError, MetadataError, MkdirError, OpenError, PathError,
-    ReadError, RmdirError, SeekError, SetMetadataError, UnlinkError, WriteError,
+    ChmodError, ChownError, CloseError, FileStatusError, MetadataError, MkdirError, OpenError,
+    PathError, ReadError, RmdirError, SeekError, SetMetadataError, UnlinkError, WriteError,
 };
 use super::{FileStatus, Mode, SeekWhence};
 use crate::utilities::anymap::AnyMap;
@@ -74,6 +74,21 @@ impl<Platform: sync::RawSyncPrimitivesProvider> FileSystem<Platform> {
             unreachable!()
         }
     }
+
+    /// Execute `f` as a specific user (for testing purposes).
+    #[cfg(test)]
+    pub fn with_user<F>(&mut self, user: u16, group: u16, f: F)
+    where
+        F: FnOnce(&mut Self),
+    {
+        let test_user = UserInfo { user, group };
+        let original_user = core::mem::replace(&mut self.current_user, test_user);
+        f(self);
+        let test_user_again = core::mem::replace(&mut self.current_user, original_user);
+        if test_user_again.user != test_user.user || test_user_again.group != test_user.group {
+            unreachable!()
+        }
+    }
 }
 
 impl<Platform: sync::RawSyncPrimitivesProvider> super::private::Sealed for FileSystem<Platform> {}
@@ -313,6 +328,48 @@ impl<Platform: sync::RawSyncPrimitivesProvider> super::FileSystem for FileSystem
         }
     }
 
+    fn chown(
+        &self,
+        path: impl crate::path::Arg,
+        user: Option<u16>,
+        group: Option<u16>,
+    ) -> Result<(), ChownError> {
+        let path = self.absolute_path(path)?;
+        let mut root = self.root.write();
+        let (_, entry) = root.parent_and_entry(&path, self.current_user)?;
+        let Some(entry) = entry else {
+            return Err(PathError::NoSuchFileOrDirectory)?;
+        };
+        match entry {
+            Entry::File(file) => {
+                let perms = &mut file.write().perms;
+                if !(self.current_user.user == 0 || self.current_user.user == perms.userinfo.user) {
+                    return Err(ChownError::NotTheOwner);
+                }
+                if let Some(new_user) = user {
+                    perms.userinfo.user = new_user;
+                }
+                if let Some(new_group) = group {
+                    perms.userinfo.group = new_group;
+                }
+                Ok(())
+            }
+            Entry::Dir(dir) => {
+                let perms = &mut dir.write().perms;
+                if !(self.current_user.user == 0 || self.current_user.user == perms.userinfo.user) {
+                    return Err(ChownError::NotTheOwner);
+                }
+                if let Some(new_user) = user {
+                    perms.userinfo.user = new_user;
+                }
+                if let Some(new_group) = group {
+                    perms.userinfo.group = new_group;
+                }
+                Ok(())
+            }
+        }
+    }
+
     fn unlink(&self, path: impl crate::path::Arg) -> Result<(), UnlinkError> {
         let path = self.absolute_path(path)?;
         let mut root = self.root.write();
diff --git a/litebox/src/fs/layered.rs b/litebox/src/fs/layered.rs
@@ -11,8 +11,8 @@ use crate::path::Arg;
 use crate::sync;
 
 use super::errors::{
-    ChmodError, CloseError, FileStatusError, MkdirError, OpenError, PathError, ReadError,
-    RmdirError, SeekError, UnlinkError, WriteError,
+    ChmodError, ChownError, CloseError, FileStatusError, MkdirError, OpenError, PathError,
+    ReadError, RmdirError, SeekError, UnlinkError, WriteError,
 };
 use super::{FileStatus, FileType, Mode, OFlags, SeekWhence};
 
@@ -623,6 +623,44 @@ impl<Platform: sync::RawSyncPrimitivesProvider, Upper: super::FileSystem, Lower:
         self.chmod(path, mode)
     }
 
+    fn chown(
+        &self,
+        path: impl crate::path::Arg,
+        user: Option<u16>,
+        group: Option<u16>,
+    ) -> Result<(), ChownError> {
+        let path = self.absolute_path(path)?;
+        match self.upper.chown(path.as_str(), user, group) {
+            Ok(()) => return Ok(()),
+            Err(e) => match e {
+                ChownError::NotTheOwner
+                | ChownError::ReadOnlyFileSystem
+                | ChownError::PathError(
+                    PathError::ComponentNotADirectory
+                    | PathError::InvalidPathname
+                    | PathError::NoSearchPerms { .. },
+                ) => {
+                    return Err(e);
+                }
+                ChownError::PathError(
+                    PathError::NoSuchFileOrDirectory | PathError::MissingComponent,
+                ) => {
+                    // fallthrough
+                }
+            },
+        }
+        self.ensure_lower_contains(&path)?;
+        match self.migrate_file_up(&path) {
+            Ok(()) => {}
+            Err(MigrationError::NoReadPerms) => unimplemented!(),
+            Err(MigrationError::NotAFile) => unimplemented!(),
+            Err(MigrationError::PathError(e)) => unreachable!(),
+        }
+        // Since it has been migrated, we can just re-trigger, causing it to apply to the
+        // upper layer
+        self.chown(path, user, group)
+    }
+
     fn unlink(&self, path: impl crate::path::Arg) -> Result<(), UnlinkError> {
         let path = self.absolute_path(path)?;
         match self.upper.unlink(path.as_str()) {
diff --git a/litebox/src/fs/mod.rs b/litebox/src/fs/mod.rs
@@ -19,8 +19,8 @@ pub mod tar_ro;
 mod tests;
 
 use errors::{
-    ChmodError, CloseError, FileStatusError, MetadataError, MkdirError, OpenError, ReadError,
-    RmdirError, SeekError, SetMetadataError, UnlinkError, WriteError,
+    ChmodError, ChownError, CloseError, FileStatusError, MetadataError, MkdirError, OpenError,
+    ReadError, RmdirError, SeekError, SetMetadataError, UnlinkError, WriteError,
 };
 
 /// A private module, to help support writing sealed traits. This module should _itself_ never be
@@ -64,6 +64,13 @@ pub trait FileSystem: private::Sealed {
     fn seek(&self, fd: &FileFd, offset: isize, whence: SeekWhence) -> Result<usize, SeekError>;
     /// Change the permissions of a file
     fn chmod(&self, path: impl path::Arg, mode: Mode) -> Result<(), ChmodError>;
+    /// Change the owner of a file
+    fn chown(
+        &self,
+        path: impl path::Arg,
+        user: Option<u16>,
+        group: Option<u16>,
+    ) -> Result<(), ChownError>;
     /// Unlink a file
     fn unlink(&self, path: impl path::Arg) -> Result<(), UnlinkError>;
     /// Create a new directory
diff --git a/litebox/src/fs/nine_p.rs b/litebox/src/fs/nine_p.rs
@@ -74,6 +74,15 @@ impl<Platform: platform::Provider> super::FileSystem for FileSystem<Platform> {
         todo!()
     }
 
+    fn chown(
+        &self,
+        path: impl crate::path::Arg,
+        user: Option<u16>,
+        group: Option<u16>,
+    ) -> Result<(), super::errors::ChownError> {
+        todo!()
+    }
+
     fn unlink(&self, path: impl crate::path::Arg) -> Result<(), super::errors::UnlinkError> {
         todo!()
     }
diff --git a/litebox/src/fs/tar_ro.rs b/litebox/src/fs/tar_ro.rs
@@ -31,8 +31,8 @@ use crate::{LiteBox, path::Arg as _, sync, utilities::anymap::AnyMap};
 use super::{
     Mode, OFlags, SeekWhence,
     errors::{
-        ChmodError, CloseError, MkdirError, OpenError, PathError, ReadError, RmdirError, SeekError,
-        UnlinkError, WriteError,
+        ChmodError, ChownError, CloseError, MkdirError, OpenError, PathError, ReadError,
+        RmdirError, SeekError, UnlinkError, WriteError,
     },
 };
 
@@ -244,6 +244,29 @@ impl<Platform: sync::RawSyncPrimitivesProvider> super::FileSystem for FileSystem
         }
     }
 
+    fn chown(
+        &self,
+        path: impl crate::path::Arg,
+        user: Option<u16>,
+        group: Option<u16>,
+    ) -> Result<(), ChownError> {
+        let path = self.absolute_path(path)?;
+        assert!(path.starts_with('/'));
+        let path = &path[1..];
+        if self
+            .tar_data
+            .entries()
+            .any(|entry| match entry.filename().as_str() {
+                Ok(p) => p == path || contains_dir(p, path),
+                Err(_) => false,
+            })
+        {
+            Err(ChownError::ReadOnlyFileSystem)
+        } else {
+            Err(PathError::NoSuchFileOrDirectory)?
+        }
+    }
+
     fn unlink(&self, path: impl crate::path::Arg) -> Result<(), UnlinkError> {
         let path = self.absolute_path(path)?;
         assert!(path.starts_with('/'));
diff --git a/litebox/src/fs/tests.rs b/litebox/src/fs/tests.rs
@@ -161,6 +161,66 @@ mod in_mem {
             "Directory should not exist"
         );
     }
+
+    #[test]
+    fn chown_test() {
+        let litebox = LiteBox::new(MockPlatform::new());
+        let mut fs = in_mem::FileSystem::new(&litebox);
+
+        // Create a test file as root
+        fs.with_root_privileges(|fs| {
+            let path = "/testfile";
+            let fd = fs
+                .open(path, OFlags::CREAT | OFlags::WRONLY, Mode::RWXU)
+                .expect("Failed to create file");
+            fs.close(fd).expect("Failed to close file");
+
+            // First chown to 1000:1000 as root (should succeed)
+            fs.chown(path, Some(1000), Some(1000))
+                .expect("Failed to chown as root");
+        });
+
+        // Switch to user 1000 and test that owner can chown (should succeed)
+        let path = "/testfile";
+        fs.with_user(1000, 1000, |fs| {
+            fs.chown(path, Some(123), Some(456))
+                .expect("Failed to chown as owner");
+        });
+
+        // Switch to a different user and test that non-owner cannot chown (should fail)
+        fs.with_user(500, 500, |fs| {
+            match fs.chown(path, Some(789), Some(101)) {
+                Err(crate::fs::errors::ChownError::NotTheOwner) => {
+                    // Expected behavior
+                }
+                Ok(()) => panic!("Non-owner should not be able to chown"),
+                Err(e) => panic!("Unexpected error: {:?}", e),
+            }
+        });
+
+        // Test chown on non-existent file (should fail)
+        match fs.chown("/nonexistent", Some(123), Some(456)) {
+            Err(crate::fs::errors::ChownError::PathError(
+                crate::fs::errors::PathError::NoSuchFileOrDirectory,
+            )) => {
+                // Expected behavior
+            }
+            Ok(()) => panic!("Should not be able to chown non-existent file"),
+            Err(e) => panic!("Unexpected error: {:?}", e),
+        }
+
+        // Test partial chown (change only user, leave group unchanged)
+        fs.with_root_privileges(|fs| {
+            fs.chown(path, Some(999), None)
+                .expect("Failed to chown user only");
+        });
+
+        // Test partial chown (change only group, leave user unchanged)
+        fs.with_root_privileges(|fs| {
+            fs.chown(path, None, Some(888))
+                .expect("Failed to chown group only");
+        });
+    }
 }
 
 mod tar_ro {
