Pass Sapling child ACL metadata through backingstore

Summary:
Use the existing parent tree child ACL metadata when building Eden TreeEntries from Sapling backingstore. Directory entries whose parent marks them as ACL roots get hasACL=true; other entries get hasACL=false. Reuse the same child ACL list for permission-denied checks so restriction handling stays aligned with the child ACL metadata.

If child ACL metadata cannot be read, fail open by logging the error and treating children as unrestricted instead of failing tree conversion.

This does not change IndexedLog/cache state; the parent positive child ACL metadata was already cached.

Reviewed By: muirdm

Differential Revision: D104246342

fbshipit-source-id: 19d21facf4b0911f4c77bda4ecd87b373caa6bd9

Author: MichaelCuevas

eden/fs/store/FilteredBackingStore.cpp

@@ -351,7 +351,8 @@ FilteredBackingStore::co_filterImpl(
         entry->second.getSize(),
         entry->second.getContentSha1(),
         entry->second.getContentBlake3(),
-        entry->second.isRestricted()};
+        entry->second.isRestricted(),
+        entry->second.hasACL()};
     pathMap.insert(std::pair{relPath.basename().copy(), std::move(treeEntry)});
   }
 

eden/fs/store/FilteredBackingStore.h

@@ -158,6 +158,7 @@ class FilteredBackingStore
   FRIEND_TEST(
       FakeSubstringFilteredBackingStoreTest,
       restrictedTreePreservedAfterFiltering);
+  FRIEND_TEST(FakeSubstringFilteredBackingStoreTest, treeEntryHasAclPreserved);
 
   ImmediateFuture<GetRootTreeResult> getRootTree(
       const RootId& rootId,

eden/fs/store/git/GitBackingStore.cpp

@@ -222,9 +222,15 @@ TreePtr GitBackingStore::getTreeImpl(const ObjectId& id) {
     }
     auto entryHash = oid2Hash(git_tree_entry_id(gitEntry));
     auto name = PathComponentPiece{entryName};
-    entries.emplace(name, std::move(entryHash), fileType);
+    entries.emplace(
+        name,
+        std::move(entryHash),
+        fileType,
+        /*isRestricted=*/false,
+        /*hasACL=*/false);
   }
-  return std::make_shared<TreePtr::element_type>(std::move(entries), id);
+  return std::make_shared<TreePtr::element_type>(
+      std::move(entries), id, AclRootState::NoAcl);
 }
 
 folly::coro::now_task<BackingStore::GetTreeAuxResult>

eden/fs/store/test/FilteredBackingStoreTest.cpp

@@ -479,6 +479,30 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, getTree) {
   EXPECT_EQ(treeOID, std::move(future5).get(0ms).tree->getObjectId());
 }
 
+TEST_F(FakeSubstringFilteredBackingStoreTest, treeEntryHasAclPreserved) {
+  Tree::container aclChildEntries{kPathMapDefaultCaseSensitive};
+  auto aclChildTree = Tree{
+      std::move(aclChildEntries), makeTestId("3004"), AclRootState::AclRoot};
+  auto rootId = makeTestId("3003");
+  auto filteredRootId =
+      FilteredObjectId(RelativePath{""}, kTestFilter1, rootId);
+  auto* rootDir = wrappedStore_->putTree(rootId, {{"acl_dir", aclChildTree}});
+  if (rootDir == nullptr) {
+    ADD_FAILURE() << "failed to create root tree";
+    return;
+  }
+
+  auto future = filteredStore_->getTree(
+      ObjectId{filteredRootId.getValue()},
+      ObjectFetchContext::getNullContext());
+  rootDir->trigger();
+  auto filteredTree = std::move(future).get(0ms).tree;
+  auto aclDir = filteredTree->find("acl_dir"_pc);
+
+  ASSERT_NE(aclDir, filteredTree->cend());
+  EXPECT_EQ(aclDir->second.hasACL(), std::optional<bool>{true});
+}
+
 TEST_F(FakeSubstringFilteredBackingStoreTest, getRootTree) {
   // Set up one commit with a root tree
   auto dir1Id = makeTestId("abc");

eden/fs/testharness/FakeBackingStore.cpp

@@ -378,7 +378,11 @@ FakeBackingStore::TreeEntryData::TreeEntryData(
     const Tree& tree)
     : entry{
           PathComponent{name},
-          TreeEntry{ObjectId{tree.getObjectId()}, TreeEntryType::TREE}} {}
+          TreeEntry{
+              ObjectId{tree.getObjectId()},
+              TreeEntryType::TREE,
+              tree.isRestricted(),
+              tree.hasACL()}} {}
 
 FakeBackingStore::TreeEntryData::TreeEntryData(
     folly::StringPiece name,
@@ -387,7 +391,9 @@ FakeBackingStore::TreeEntryData::TreeEntryData(
           PathComponent{name},
           TreeEntry{
               ObjectId{tree->get().getObjectId()},
-              TreeEntryType::TREE}} {}
+              TreeEntryType::TREE,
+              tree->get().isRestricted(),
+              tree->get().hasACL()}} {}
 
 StoredTree* FakeBackingStore::putTree(
     const std::initializer_list<TreeEntryData>& entryArgs) {

eden/scm/lib/backingstore/include/ffi.h

@@ -11,7 +11,9 @@
 #include <folly/container/FBVector.h>
 #include <folly/futures/Future.h>
 #include <rust/cxx.h>
+#include <cstdint>
 #include <optional>
+#include <stdexcept>
 #include <string_view>
 #include <utility>
 
@@ -127,7 +129,8 @@ class TreeBuilder {
       rust::Str name,
       const std::array<uint8_t, 20>& hg_node,
       facebook::eden::TreeEntryType ttype,
-      bool is_restricted);
+      bool is_restricted,
+      bool has_acl);
 
   // Add tree entry with aux data.
   void add_entry_with_aux_data(
@@ -137,7 +140,8 @@ class TreeBuilder {
       const uint64_t size,
       const std::array<uint8_t, 20>& sha1,
       const std::array<uint8_t, 32>& blake3,
-      bool is_restricted);
+      bool is_restricted,
+      bool has_acl);
 
   // Set aux data for tree itself (if available).
   void set_aux_data(const std::array<uint8_t, 32>& digest, uint64_t size);

eden/scm/lib/backingstore/src/ffi.cpp

@@ -92,16 +92,14 @@ void TreeBuilder::add_entry(
     rust::Str name,
     const std::array<uint8_t, 20>& hg_node,
     facebook::eden::TreeEntryType ttype,
-    bool is_restricted) {
+    bool is_restricted,
+    bool has_acl) {
   emplace_entry(
       name,
       facebook::eden::TreeEntry{
           make_entry_oid(hg_node, name),
           ttype,
-          std::nullopt,
-          std::nullopt,
-          std::nullopt,
-          is_restricted,
+          facebook::eden::makeAclRootState(is_restricted, has_acl),
       });
 }
 
@@ -112,7 +110,8 @@ void TreeBuilder::add_entry_with_aux_data(
     const uint64_t size,
     const std::array<uint8_t, 20>& sha1,
     const std::array<uint8_t, 32>& blake3,
-    bool is_restricted) {
+    bool is_restricted,
+    bool has_acl) {
   emplace_entry(
       name,
       facebook::eden::TreeEntry{
@@ -121,7 +120,7 @@ void TreeBuilder::add_entry_with_aux_data(
           size,
           std::optional<facebook::eden::Hash20>(sha1),
           std::optional<facebook::eden::Hash32>(blake3),
-          is_restricted,
+          facebook::eden::makeAclRootState(is_restricted, has_acl),
       });
 }
 

eden/scm/lib/backingstore/src/ffi.rs

@@ -25,6 +25,7 @@ use storemodel::TreeItemFlag;
 use types::FetchContext;
 use types::HgId;
 use types::Key;
+use types::PathComponentBuf;
 use types::RepoPath;
 use types::fetch_cause::FetchCause;
 use types::fetch_mode::FetchMode;
@@ -206,6 +207,7 @@ pub(crate) mod ffi {
             hg_node: &[u8; 20],
             ttype: TreeEntryType,
             is_restricted: bool,
+            has_acl: bool,
         );
 
         fn add_entry_with_aux_data(
@@ -217,6 +219,7 @@ pub(crate) mod ffi {
             sha1: &[u8; 20],
             blake3: &[u8; 32],
             is_restricted: bool,
+            has_acl: bool,
         );
 
         fn mark_missing(self: Pin<&mut TreeBuilder>);
@@ -510,48 +513,63 @@ pub fn sapling_backingstore_get_tree(
     ffi::GetTreeResult { error }
 }
 
-/// Build a set of restricted child directory HgIds from the tree's
-/// permission_denied_children(). Fail-open: if the initial call or
-/// individual entries error, log a warning and treat as unrestricted.
-fn build_restricted_set(tree: &dyn TreeEntry) -> HashSet<HgId> {
-    let iter = match tree.permission_denied_children() {
+/// Build a set of restricted child directory HgIds from known ACL children.
+/// Fail-open: if the initial check or individual entries error, log a warning
+/// and treat as unrestricted.
+fn build_restricted_set(
+    tree: &dyn TreeEntry,
+    children_with_acl: Vec<(PathComponentBuf, HgId)>,
+) -> HashSet<HgId> {
+    let iter = match tree.filter_permission_denied(children_with_acl) {
         Ok(iter) => iter,
         Err(e) => {
             tracing::warn!(
                 ?e,
-                "error calling permission_denied_children; treating all children as unrestricted (fail-open)"
+                "error checking restricted children; treating all children as unrestricted (fail-open)"
             );
             return HashSet::new();
         }
     };
-    iter.filter_map(|r| match r {
+
+    iter.filter_map(|entry| match entry {
         Ok((_, hgid, _)) => Some(hgid),
         Err(e) => {
-            tracing::warn!(?e, "error checking permission_denied_children");
+            tracing::warn!(
+                ?e,
+                "error checking restricted child; treating child as unrestricted (fail-open)"
+            );
             None
         }
     })
     .collect()
 }
 
+fn build_acl_sets(tree: &dyn TreeEntry) -> (HashSet<HgId>, HashSet<HgId>) {
+    let children_with_acl = match tree.children_with_acls() {
+        Ok(children) => children,
+        Err(e) => {
+            tracing::warn!(
+                ?e,
+                "error reading child ACL metadata; treating all children as unrestricted (fail-open)"
+            );
+            return (HashSet::new(), HashSet::new());
+        }
+    };
+    let child_has_acl = children_with_acl.iter().map(|(_, hgid)| *hgid).collect();
+    let restricted_set = build_restricted_set(tree, children_with_acl);
+    (child_has_acl, restricted_set)
+}
+
 // Convert the `TreeEntry` trait object into an EdenFS Tree by adding each entry to the TreeBuilder
 // object.
 fn add_tree_to_builder(
     mut builder: Pin<&mut ffi::TreeBuilder>,
     tree: Arc<dyn TreeEntry>,
 ) -> anyhow::Result<()> {
-    // TODO: Make the aux data available in `TreeEntry::iter()` so we don't have to do this HashMap business.
     let aux_map: HashMap<HgId, ScmStoreFileAuxData> =
         tree.file_aux_iter()?.collect::<anyhow::Result<_>>()?;
 
-    // Build a set of child directory IDs that the server denied access to
-    // (path ACL restriction). These are directories containing .slacl files.
-    //
-    // Naming mapping (B2): The Sapling layer uses "has_acl" / "permission_denied"
-    // to describe directories with access restrictions. EdenFS translates this
-    // to "is_restricted" / "isRestricted" to describe the access-denied behavior
-    // from the user's perspective.
-    let restricted_set: HashSet<HgId> = build_restricted_set(tree.as_ref());
+    let (child_has_acl, restricted_set) = build_acl_sets(tree.as_ref());
 
     // Pre-allocate the per-entry storage.
     if let Some(hint) = tree.size_hint() {
@@ -575,6 +593,7 @@ fn add_tree_to_builder(
 
         let is_restricted =
             matches!(flag, TreeItemFlag::Directory) && restricted_set.contains(&node);
+        let has_acl = matches!(flag, TreeItemFlag::Directory) && child_has_acl.contains(&node);
 
         if let Some(aux) = aux_map.get(&node) {
             builder.as_mut().add_entry_with_aux_data(
@@ -585,11 +604,16 @@ fn add_tree_to_builder(
                 aux.sha1.as_byte_array(),
                 aux.blake3.as_byte_array(),
                 is_restricted,
+                has_acl,
             );
         } else {
-            builder
-                .as_mut()
-                .add_entry(name.as_str(), node.as_byte_array(), ttype, is_restricted);
+            builder.as_mut().add_entry(
+                name.as_str(),
+                node.as_byte_array(),
+                ttype,
+                is_restricted,
+                has_acl,
+            );
         }
     }
 

eden/scm/lib/backingstore/test/TreeBuilderTest.cpp

@@ -73,7 +73,14 @@ TEST_F(TreeBuilderTest, AddFileEntry) {
       0xf7, 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff, 0x00};
 
   builder.add_entry_with_aux_data(
-      fileName, hgNode, entryType, fileSize, sha1Hash, blake3Hash, false);
+      fileName,
+      hgNode,
+      entryType,
+      fileSize,
+      sha1Hash,
+      blake3Hash,
+      false,
+      false);
 
   EXPECT_EQ(builder.num_files(), 1);
   EXPECT_EQ(builder.num_dirs(), 0);
@@ -107,7 +114,7 @@ TEST_F(TreeBuilderTest, AddDirectoryEntry) {
   TreeEntryType entryType = TreeEntryType::TREE;
 
   // Add the directory entry (we don't currently support dir aux data):
-  builder.add_entry(dirName, hgNode, entryType, false);
+  builder.add_entry(dirName, hgNode, entryType, false, true);
 
   EXPECT_EQ(builder.num_files(), 0);
   EXPECT_EQ(builder.num_dirs(), 1);
@@ -118,12 +125,39 @@ TEST_F(TreeBuilderTest, AddDirectoryEntry) {
   auto entry = tree->find(PathComponentPiece{"test_dir"})->second;
 
   EXPECT_EQ(entry.getType(), entryType);
+  EXPECT_EQ(entry.aclRootState(), AclRootState::AclRoot);
+  EXPECT_EQ(entry.hasACL(), true);
 
   SlOid parsedOid = facebook::eden::SlOid{entry.getObjectId()};
   EXPECT_EQ(parsedOid.node().getBytes(), folly::ByteRange{hgNode});
   EXPECT_EQ(parsedOid.path(), path_ + PathComponentPiece{"test_dir"});
 }
 
+TEST_F(TreeBuilderTest, AddDirectoryAclRootStates) {
+  TreeBuilder builder(oid_, caseSensitive_, objectIdFormat_);
+
+  std::array<uint8_t, 20> noAclNode = {0x01};
+  std::array<uint8_t, 20> permittedNode = {0x02};
+  std::array<uint8_t, 20> restrictedNode = {0x03};
+
+  builder.add_entry("no_acl", noAclNode, TreeEntryType::TREE, false, false);
+  builder.add_entry(
+      "permitted", permittedNode, TreeEntryType::TREE, false, true);
+  builder.add_entry(
+      "restricted", restrictedNode, TreeEntryType::TREE, true, true);
+
+  auto tree = builder.build();
+  EXPECT_EQ(
+      tree->find(PathComponentPiece{"no_acl"})->second.aclRootState(),
+      AclRootState::NoAcl);
+  EXPECT_EQ(
+      tree->find(PathComponentPiece{"permitted"})->second.aclRootState(),
+      AclRootState::AclRoot);
+  EXPECT_EQ(
+      tree->find(PathComponentPiece{"restricted"})->second.aclRootState(),
+      AclRootState::RestrictedAclRoot);
+}
+
 TEST_F(TreeBuilderTest, AddMultipleEntries) {
   TreeBuilder builder(oid_, caseSensitive_, objectIdFormat_);
 
@@ -134,7 +168,8 @@ TEST_F(TreeBuilderTest, AddMultipleEntries) {
     std::array<uint8_t, 20> hgNode = {0};
     hgNode[0] = static_cast<uint8_t>(i);
 
-    builder.add_entry(fileNameStr, hgNode, TreeEntryType::REGULAR_FILE, false);
+    builder.add_entry(
+        fileNameStr, hgNode, TreeEntryType::REGULAR_FILE, false, false);
   }
 
   // Add multiple directory entries
@@ -144,7 +179,7 @@ TEST_F(TreeBuilderTest, AddMultipleEntries) {
     std::array<uint8_t, 20> hgNode = {0};
     hgNode[1] = static_cast<uint8_t>(i);
 
-    builder.add_entry(dirNameStr, hgNode, TreeEntryType::TREE, false);
+    builder.add_entry(dirNameStr, hgNode, TreeEntryType::TREE, false, false);
   }
 
   EXPECT_EQ(builder.num_files(), 3);