[package-directory] Rework tests to use VFS connections

Ensure package directory tests serve a connection to the directories
being tested, and all child nodes are opened through fuchsia.io. This
ensures that the VFS connection logic (including hierarchal rights
enforcement and path handling) is also tested. This results in a
few test-only changes:

 * Hierarchal rights are now enforced, so in cases where NOT_SUPPORTED
   was being checked before, we now need ACCESS_DENIED.
 * The VFS handles checking certain flags already reducing the verbosity
   of these tests. This includes attempts to create new nodes, which
   requires the parent connection to be served as writable. This is not
   possible with how the nodes are implemented, so these tests have been
   removed in lieu of assertions that guarantee we cannot create
   new files, and tests that ensure we cannot create mutable connections
 * Callers of the fuchsia.io/Directory.DeprecatedOpen method now reject
   requests to open files where the path ends with a trailing slash,
   since the VFS adds OpenFlags.DIRECTORY in these cases. This is NOT
   done for the new fuchsia.io/Directory.Open method, so that is still
   allowed for those tests.

As we are now testing through a DirectoryProxy instead of calling the
VFS Node trait methods directly, we can simplify a lot of test code by
using the helpers in the fuchsia-fs crate. This improves test
readability significantly in some cases, and also ensures that we test
end-to-end behavior that clients will encounter.

There should be no change to code coverage with these tests, but there
is a small functional change: in certain cases when calling
fuchsia.io/Directory.DeprecatedOpen, the APPEND flag was erroneously
allowed. We cannot easily fix this without disabling the old CTF tests,
so for now we just update them going forwards and leave the old
behavior. We need to address this as part of the io2 migration soon.

Remove redundant test cases where the VFS already enforces certain
checks, and instead add assertions which ensure no regressions in
behavior can occur.

Bug: 324111518
Test: fx test package-directory-tests
Change-Id: Idac8441d889ffdec3a3f93dc92ab0c7b0aaff036
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1234244
Reviewed-by: Ben Keller <[email protected]>
Commit-Queue: Brandon Castellano <[email protected]>

Author: Breakthrough

src/sys/pkg/lib/package-directory/src/lib.rs

@@ -24,11 +24,13 @@ mod root_dir_cache;
 pub use root_dir::{PathError, ReadFileError, RootDir, SubpackagesError};
 pub use root_dir_cache::RootDirCache;
 pub use vfs::execution_scope::ExecutionScope;
-pub use vfs::path::Path as VfsPath;
 
 pub(crate) const DIRECTORY_ABILITIES: fio::Abilities =
     fio::Abilities::GET_ATTRIBUTES.union(fio::Abilities::ENUMERATE).union(fio::Abilities::TRAVERSE);
 
+pub(crate) const MUTABLE_FLAGS: fio::Flags =
+    fio::Flags::PERM_MODIFY.union(fio::Flags::PERM_SET_ATTRIBUTES).union(fio::Flags::PERM_WRITE);
+
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
     #[error("the meta.far was not found")]
@@ -257,7 +259,7 @@ pub fn serve(
         non_meta_storage,
         meta_far,
         flags,
-        VfsPath::dot(),
+        vfs::Path::dot(),
         server_end.into_channel().into(),
     )
 }
@@ -273,7 +275,7 @@ pub async fn serve_path(
     non_meta_storage: impl NonMetaStorage,
     meta_far: fuchsia_hash::Hash,
     flags: fio::Flags,
-    path: VfsPath,
+    path: vfs::Path,
     server_end: ServerEnd<fio::NodeMarker>,
 ) -> Result<(), Error> {
     let root_dir = match RootDir::new(non_meta_storage, meta_far).await {
@@ -365,9 +367,6 @@ mod tests {
     use fuchsia_pkg_testing::blobfs::Fake as FakeBlobfs;
     use fuchsia_pkg_testing::PackageBuilder;
     use futures::StreamExt;
-    use std::any::Any;
-    use std::sync::Arc;
-    use vfs::directory::dirents_sink::{self, AppendResult, Sealed, Sink};
     use vfs::directory::helper::DirectlyMutable;
 
     #[fuchsia_async::run_singlethreaded(test)]
@@ -410,7 +409,7 @@ mod tests {
             blobfs_client,
             metafar_blob.merkle,
             fio::PERM_READABLE,
-            VfsPath::validate_and_split(".").unwrap(),
+            vfs::Path::validate_and_split(".").unwrap(),
             server_end.into_channel().into(),
         )
         .await
@@ -438,7 +437,7 @@ mod tests {
             blobfs_client,
             metafar_blob.merkle,
             fio::PERM_READABLE | fio::Flags::PROTOCOL_FILE,
-            VfsPath::validate_and_split("meta").unwrap(),
+            vfs::Path::validate_and_split("meta").unwrap(),
             server_end.into_channel().into(),
         )
         .await
@@ -464,7 +463,7 @@ mod tests {
                 blobfs_client,
                 metafar_blob.merkle,
                 fio::PERM_READABLE | fio::Flags::FLAG_SEND_REPRESENTATION,
-                VfsPath::validate_and_split("not-present").unwrap(),
+                vfs::Path::validate_and_split("not-present").unwrap(),
                 server_end.into_channel().into(),
             )
             .await,
@@ -487,7 +486,7 @@ mod tests {
                 blobfs_client,
                 Hash::from([0u8; 32]),
                 fio::PERM_READABLE | fio::Flags::FLAG_SEND_REPRESENTATION,
-                VfsPath::validate_and_split(".").unwrap(),
+                vfs::Path::validate_and_split(".").unwrap(),
                 server_end.into_channel().into(),
             )
             .await,
@@ -561,77 +560,17 @@ mod tests {
         assert_eq!(get_dir_children(["a/b/c/d"], "a/"), vec![(dir(), "b".to_string())]);
     }
 
-    /// Implementation of vfs::directory::dirents_sink::Sink.
-    /// Sink::append begins to fail (returns Sealed) after `max_entries` entries have been appended.
-    #[derive(Clone)]
-    pub(crate) struct FakeSink {
-        max_entries: usize,
-        pub(crate) entries: Vec<(String, EntryInfo)>,
-        sealed: bool,
-    }
-
-    impl FakeSink {
-        pub(crate) fn new(max_entries: usize) -> Self {
-            FakeSink { max_entries, entries: Vec::with_capacity(max_entries), sealed: false }
-        }
-
-        pub(crate) fn from_sealed(sealed: Box<dyn dirents_sink::Sealed>) -> Box<FakeSink> {
-            sealed.into()
-        }
-    }
-
-    impl From<Box<dyn dirents_sink::Sealed>> for Box<FakeSink> {
-        fn from(sealed: Box<dyn dirents_sink::Sealed>) -> Self {
-            sealed.open().downcast::<FakeSink>().unwrap()
-        }
-    }
-
-    impl Sink for FakeSink {
-        fn append(mut self: Box<Self>, entry: &EntryInfo, name: &str) -> AppendResult {
-            assert!(!self.sealed);
-            if self.entries.len() == self.max_entries {
-                AppendResult::Sealed(self.seal())
-            } else {
-                self.entries.push((name.to_owned(), entry.clone()));
-                AppendResult::Ok(self)
-            }
-        }
-
-        fn seal(mut self: Box<Self>) -> Box<dyn Sealed> {
-            self.sealed = true;
-            self
-        }
-    }
-
-    impl Sealed for FakeSink {
-        fn open(self: Box<Self>) -> Box<dyn Any> {
-            self
-        }
-    }
-
     const BLOB_CONTENTS: &[u8] = b"blob-contents";
 
     fn blob_contents_hash() -> Hash {
         fuchsia_merkle::from_slice(BLOB_CONTENTS).root()
     }
 
-    fn serve_directory(directory: Arc<vfs::directory::immutable::Simple>) -> fio::DirectoryProxy {
-        let (proxy, server_end) = fidl::endpoints::create_proxy::<fio::DirectoryMarker>();
-        vfs::directory::entry_container::Directory::open(
-            directory,
-            vfs::execution_scope::ExecutionScope::new(),
-            fio::OpenFlags::RIGHT_READABLE,
-            vfs::path::Path::dot(),
-            fidl::endpoints::ServerEnd::new(server_end.into_channel()),
-        );
-        proxy
-    }
-
     #[fuchsia_async::run_singlethreaded(test)]
     async fn bootfs_get_vmo_blob() {
         let directory = vfs::directory::immutable::simple();
         directory.add_entry(blob_contents_hash(), vfs::file::read_only(BLOB_CONTENTS)).unwrap();
-        let proxy = serve_directory(directory);
+        let proxy = vfs::directory::serve_read_only(directory);
 
         let vmo = proxy.get_blob_vmo(&blob_contents_hash()).await.unwrap();
         assert_eq!(vmo.read_to_vec(0, BLOB_CONTENTS.len() as u64).unwrap(), BLOB_CONTENTS);
@@ -641,15 +580,15 @@ mod tests {
     async fn bootfs_read_blob() {
         let directory = vfs::directory::immutable::simple();
         directory.add_entry(blob_contents_hash(), vfs::file::read_only(BLOB_CONTENTS)).unwrap();
-        let proxy = serve_directory(directory);
+        let proxy = vfs::directory::serve_read_only(directory);
 
         assert_eq!(proxy.read_blob(&blob_contents_hash()).await.unwrap(), BLOB_CONTENTS);
     }
 
     #[fuchsia_async::run_singlethreaded(test)]
     async fn bootfs_get_vmo_blob_missing_blob() {
         let directory = vfs::directory::immutable::simple();
-        let proxy = serve_directory(directory);
+        let proxy = vfs::directory::serve_read_only(directory);
 
         let result = proxy.get_blob_vmo(&blob_contents_hash()).await;
         assert_matches!(result, Err(NonMetaStorageError::OpenBlob(e)) if e.is_not_found_error());
@@ -658,7 +597,7 @@ mod tests {
     #[fuchsia_async::run_singlethreaded(test)]
     async fn bootfs_read_blob_missing_blob() {
         let directory = vfs::directory::immutable::simple();
-        let proxy = serve_directory(directory);
+        let proxy = vfs::directory::serve_read_only(directory);
 
         let result = proxy.read_blob(&blob_contents_hash()).await;
         assert_matches!(result, Err(NonMetaStorageError::ReadBlob(e)) if e.is_not_found_error());