fix(acl): skip a dirent with no Authority kind instead of hiding the page

common/cf-acl.c

@@ -92,18 +92,28 @@ filter_chunk (const char *repo, const char *dir, const char *user,
     json_t *req=json_object(), *checks=json_array(), *reply=NULL;
     GString *body=g_string_new(NULL);
     char *payload=NULL;
+    /* Same order as checks: an entry without an Authority kind is skipped,
+     * so position i of permissions no longer maps to position i of begin. */
+    GList *included=NULL;
     if (!req || !checks || !body || !repo || !dir || !user) goto done;
     for (GList *p=begin; p && count > 0; p=p->next, --count) {
         GObject *item=p->data;
         char *name=NULL;
         int mode=0;
         g_object_get(item,"obj_name",&name,"mode",&mode,NULL);
         if (!name || !*name || strchr(name,'/') ||
-            !strcmp(name,".") || !strcmp(name,"..") ||
-            (!S_ISDIR(mode) && !S_ISREG(mode))) {
+            !strcmp(name,".") || !strcmp(name,"..")) {
             g_free(name);
             goto done;
         }
+        /* A dirent that is neither a regular file nor a directory (a native
+         * symlink, for example) has no Authority kind. Skip that child only:
+         * aborting the chunk would hide every sibling on the page. A hidden
+         * child simply gets no permission below, like a denied sibling. */
+        if (!S_ISDIR(mode) && !S_ISREG(mode)) {
+            g_free(name);
+            continue;
+        }
         char *path=g_build_filename(dir,name,NULL);
         g_free(name);
         /* File rights follow the containing directory; a directory entry
@@ -117,9 +127,13 @@ filter_chunk (const char *repo, const char *dir, const char *user,
             goto done;
         }
         json_decref(check);
+        included=g_list_append(included,item);
     }
     unsigned size=(unsigned)json_array_size(checks);
-    if (!size || size>512 ||
+    /* Every child was skipped: nothing to authorize, the page stays empty
+     * instead of failing closed over the whole directory. */
+    if (!size) { result=0; goto done; }
+    if (size>512 ||
         json_object_set_new(req,"repo_id",json_string(repo)) ||
         json_object_set_new(req,"username",json_string(user)) ||
         json_object_set_new(req,"partial",json_true()) ||
@@ -131,14 +145,14 @@ filter_chunk (const char *repo, const char *dir, const char *user,
     if (!json_is_true(json_object_get(reply,"allowed")) ||
         g_strcmp0(json_string_value(json_object_get(reply,"contract")),"cf-acl-v1") ||
         !json_is_array(permissions) || json_array_size(permissions)!=size) goto done;
-    GList *p=begin;
+    GList *p=included;
     for (unsigned i=0;i<size;++i,p=p->next) {
         const char *permission=json_string_value(json_array_get(permissions,i));
         if (!json_is_null(json_array_get(permissions,i)) &&
             (!permission || (strcmp(permission,"r") && strcmp(permission,"rw"))))
             goto done;
     }
-    p=begin;
+    p=included;
     for (unsigned i=0;i<size;++i,p=p->next) {
         const char *permission=json_string_value(json_array_get(permissions,i));
         if (permission) {
@@ -149,6 +163,7 @@ filter_chunk (const char *repo, const char *dir, const char *user,
     }
     result=0;
 done:
+    g_list_free(included);
     json_decref(reply);json_decref(req);json_decref(checks);
     g_free(payload);
     if(body)g_string_free(body,TRUE);

tests/test_cf_acl_dirent_kind.py

@@ -1,18 +1,53 @@
-"""Cheap contract: native batch sends file vs directory kind to Hub Authority."""
+"""Cheap contract: native batch sends file vs directory kind to Hub Authority.
+
+An entry with no Authority kind (a native symlink) must be skipped on its own:
+aborting the chunk hides every sibling on the page.
+"""
 from pathlib import Path
 import unittest
 
+ROOT = Path(__file__).resolve().parents[1]
+
+
+def chunk():
+    source = (ROOT / 'common' / 'cf-acl.c').read_text(encoding='utf-8')
+    return source.split('filter_chunk (', 1)[1].split('\nGList *\ncf_acl_filter_dirents', 1)[0]
+
 
 class NativeDirentKindTests(unittest.TestCase):
     def test_native_permission_batch_uses_real_dirent_mode(self):
-        root = Path(__file__).resolve().parents[1]
-        source = (root / 'common' / 'cf-acl.c').read_text(encoding='utf-8')
-        chunk = source.split('filter_chunk (', 1)[1].split('\nGList *\ncf_acl_filter_dirents', 1)[0]
-        self.assertIn('"mode",&mode', chunk)
-        self.assertIn('S_ISDIR(mode) ? "dir" : "file"', chunk)
-        self.assertIn('"path",path,"kind",kind,"action","read"', chunk)
-        self.assertIn('cf_v2_acl_check_json(seaf,payload,body)', chunk)
-        self.assertIn('json_array_size(permissions)!=size', chunk)
+        text = chunk()
+        self.assertIn('"mode",&mode', text)
+        self.assertIn('S_ISDIR(mode) ? "dir" : "file"', text)
+        self.assertIn('"path",path,"kind",kind,"action","read"', text)
+        self.assertIn('cf_v2_acl_check_json(seaf,payload,body)', text)
+        self.assertIn('json_array_size(permissions)!=size', text)
+
+    def test_invalid_name_still_aborts_the_chunk(self):
+        text = chunk()
+        name_guard = text.split('g_object_get(item,"obj_name",&name,"mode",&mode,NULL);', 1)[1]
+        name_guard = name_guard.split('/* A dirent that is neither', 1)[0]
+        self.assertIn('strchr(name,\'/\')', name_guard)
+        self.assertIn('goto done;', name_guard)
+
+    def test_unknown_dirent_kind_skips_only_that_child(self):
+        text = chunk()
+        mode_guard = text.split('if (!S_ISDIR(mode) && !S_ISREG(mode)) {', 1)[1].split('}', 1)[0]
+        self.assertIn('continue;', mode_guard)
+        self.assertNotIn('goto done', mode_guard)
+
+    def test_permissions_map_to_the_checked_children_only(self):
+        text = chunk()
+        mapping = text.split('json_array_size(permissions)!=size', 1)[1].split('result=0;', 1)[0]
+        self.assertIn('included=g_list_append(included,item);', text)
+        self.assertIn('GList *p=included;', mapping)
+        self.assertEqual(mapping.count('p=included;'), 2)
+        self.assertNotIn('GList *p=begin;', mapping)
+        self.assertIn('g_list_free(included);', text)
+
+    def test_all_children_skipped_is_not_a_batch_failure(self):
+        text = chunk()
+        self.assertIn('if (!size) { result=0; goto done; }', text)
 
 
 if __name__ == '__main__':