all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [RFC PATCH access-control 1/4] acl: support 'additive' option on ACL entries
Date: Thu, 16 Jul 2026 01:19:59 +0200	[thread overview]
Message-ID: <20260715232121.1009607-2-t.lamprecht@proxmox.com> (raw)
In-Reply-To: <20260715232121.1009607-1-t.lamprecht@proxmox.com>

By default a more specific ACL entry on a deeper path fully replaces
the roles inherited from ancestor paths. That is what you want for
scoped delegation, but not for broad grants that should hold across
the whole tree, like an auditor group that must keep read access no
matter what narrower ACLs exist below it.

Add an optional 'additive' flag to ACL entries. An additive entry's
roles are merged into those of deeper entries instead of being
replaced by them, so the roles in effect at a path are the union of
that path's own roles and any additive roles inherited from above.
NoAccess on an additive entry still blocks everything below it.

Store the flag per role, not just per member, so it maps one-to-one
to a config line; otherwise a second role on the same member could
be turned additive by accident when the config is written back out.
Additive only makes sense together with propagate, so reject it
without, and let the parser warn and drop the flag if it meets that
combination in a hand-edited config.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/AccessControl.pm  | 247 +++++++++++++++++++++++++++++---------
 src/test/Makefile         |   1 +
 src/test/parser_writer.pl | 149 +++++++++++++++++++++++
 src/test/perm-test9.pl    | 181 ++++++++++++++++++++++++++++
 src/test/test9.cfg        |  86 +++++++++++++
 5 files changed, 606 insertions(+), 58 deletions(-)
 create mode 100644 src/test/perm-test9.pl
 create mode 100644 src/test/test9.cfg

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 0d632b3..01dbfd6 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1000,6 +1000,22 @@ sub find_acl_tree_node {
     return $node;
 }
 
+# Set or clear the per-role additive flag on an ACL node. Clearing prunes the sparse
+# 'additive_{users,groups,tokens}' hashes so writers and the API never see a stale entry.
+sub update_acl_additive {
+    my ($acl_node, $type, $id, $role, $additive) = @_;
+
+    if ($additive) {
+        $acl_node->{$type}->{$id}->{$role} = 1;
+        return;
+    }
+    return if !$acl_node->{$type} || !$acl_node->{$type}->{$id};
+
+    delete($acl_node->{$type}->{$id}->{$role});
+    delete($acl_node->{$type}->{$id}) if !%{ $acl_node->{$type}->{$id} };
+    delete($acl_node->{$type}) if !%{ $acl_node->{$type} };
+}
+
 sub add_user_group {
     my ($username, $usercfg, $group) = @_;
 
@@ -1025,6 +1041,8 @@ sub delete_user_acl {
 
         delete($acl_node->{users}->{$username})
             if $acl_node->{users}->{$username};
+        delete($acl_node->{additive_users}->{$username})
+            if $acl_node->{additive_users} && $acl_node->{additive_users}->{$username};
     };
 
     iterate_acl_tree("/", $usercfg->{acl_root}, $code);
@@ -1038,6 +1056,8 @@ sub delete_group_acl {
 
         delete($acl_node->{groups}->{$group})
             if $acl_node->{groups}->{$group};
+        delete($acl_node->{additive_groups}->{$group})
+            if $acl_node->{additive_groups} && $acl_node->{additive_groups}->{$group};
     };
 
     iterate_acl_tree("/", $usercfg->{acl_root}, $code);
@@ -1509,10 +1529,31 @@ sub parse_user_config {
             }
 
         } elsif ($et eq 'acl') {
-            my ($propagate, $pathtxt, $uglist, $rolelist) = @data;
+            my ($propagate, $pathtxt, $uglist, $rolelist, $options) = @data;
 
             $propagate = $propagate ? 1 : 0;
 
+            # The on-disk option for 'additive' is '+', to keep ACL lines compact; the API and
+            # tooling use the long name 'additive'.
+            my $additive = 0;
+            if ($options) {
+                for my $opt (split_list($options)) {
+                    if ($opt eq '+') {
+                        $additive = 1;
+                    } else {
+                        warn "user config - ignore unknown acl option '$opt'\n";
+                    }
+                }
+            }
+
+            # A non-propagating entry has nothing to merge down, so an 'additive' flag on it is
+            # meaningless: strip it with a warning. The API rejects the same combo up front.
+            if ($additive && !$propagate) {
+                warn "user config - 'additive' requires 'propagate' for acl entry"
+                    . " at path '$pathtxt', ignoring flag\n";
+                $additive = 0;
+            }
+
             if (my $path = normalize_path($pathtxt)) {
                 my $acl_node;
                 foreach my $role (split_list($rolelist)) {
@@ -1527,6 +1568,18 @@ sub parse_user_config {
                         next;
                     }
 
+                    # Warn if a duplicate line for the same (member, role) at this path sets
+                    # conflicting flags; the last line still wins, but the conflict is surfaced.
+                    my $warn_conflict = sub {
+                        my ($roles_hash, $add_hash, $label, $id) = @_;
+                        return if !defined($roles_hash->{$id}->{$role});
+                        my $prev_prop = $roles_hash->{$id}->{$role};
+                        my $prev_add = $add_hash->{$id}->{$role} ? 1 : 0;
+                        return if $prev_prop == $propagate && $prev_add == $additive;
+                        warn "user config - duplicate acl line for $label '$id' role '$role'"
+                            . " at '$pathtxt' with conflicting flags; last line wins\n";
+                    };
+
                     foreach my $ug (split_list($uglist)) {
                         my ($group) = $ug =~ m/^@(\S+)$/;
 
@@ -1535,18 +1588,43 @@ sub parse_user_config {
                                 warn "user config - ignore invalid acl group '$group'\n";
                             }
                             $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node;
+                            $warn_conflict->(
+                                $acl_node->{groups},
+                                $acl_node->{additive_groups} // {},
+                                'group',
+                                $group,
+                            );
                             $acl_node->{groups}->{$group}->{$role} = $propagate;
+                            update_acl_additive(
+                                $acl_node, 'additive_groups', $group, $role, $additive,
+                            );
                         } elsif (PVE::Auth::Plugin::verify_username($ug, 1)) {
                             if (!$cfg->{users}->{$ug}) { # user does not exist
                                 warn "user config - ignore invalid acl member '$ug'\n";
                             }
                             $acl_node = find_acl_tree_node($cfg->{acl_root}, $path) if !$acl_node;
+                            $warn_conflict->(
+                                $acl_node->{users},
+                                $acl_node->{additive_users} // {},
+                                'user',
+                                $ug,
+                            );
                             $acl_node->{users}->{$ug}->{$role} = $propagate;
+                            update_acl_additive($acl_node, 'additive_users', $ug, $role, $additive);
                         } elsif (my ($user, $token) = split_tokenid($ug, 1)) {
                             if (check_token_exist($cfg, $user, $token, 1)) {
                                 $acl_node = find_acl_tree_node($cfg->{acl_root}, $path)
                                     if !$acl_node;
+                                $warn_conflict->(
+                                    $acl_node->{tokens},
+                                    $acl_node->{additive_tokens} // {},
+                                    'token',
+                                    $ug,
+                                );
                                 $acl_node->{tokens}->{$ug}->{$role} = $propagate;
+                                update_acl_additive(
+                                    $acl_node, 'additive_tokens', $ug, $role, $additive,
+                                );
                             } else {
                                 warn "user config - ignore invalid acl token '$ug'\n";
                             }
@@ -1707,26 +1785,32 @@ sub write_user_config {
 
     $data .= "\n";
 
+    # Bucket a member's roles by (propagate, additive) so each flag combination emits as its own
+    # acl line; otherwise a non-additive role sharing a (member, path) would be silently promoted
+    # to additive on the next write.
     my $collect_rolelist_members = sub {
-        my ($acl_members, $result, $prefix, $exclude) = @_;
+        my ($acl_members, $additive_members, $result, $prefix, $exclude) = @_;
 
         foreach my $member (keys %$acl_members) {
             next if $exclude && $member eq $exclude;
 
-            my $l0 = '';
-            my $l1 = '';
+            my $member_additive = $additive_members->{$member} // {};
+
+            my $buckets = {};
             foreach my $role (sort keys %{ $acl_members->{$member} }) {
                 my $propagate = $acl_members->{$member}->{$role};
-                if ($propagate) {
-                    $l1 .= ',' if $l1;
-                    $l1 .= $role;
-                } else {
-                    $l0 .= ',' if $l0;
-                    $l0 .= $role;
+                # The parser rejects 'additive' on a non-propagating entry, so strip it here too
+                # to keep the written config round-trip-stable even if in-memory state drifted.
+                my $additive = ($propagate && $member_additive->{$role}) ? 1 : 0;
+                $buckets->{$propagate}->{$additive}->{$role} = 1;
+            }
+            for my $propagate (keys %$buckets) {
+                for my $additive (keys %{ $buckets->{$propagate} }) {
+                    my $rolelist =
+                        join(',', sort keys %{ $buckets->{$propagate}->{$additive} });
+                    $result->{$propagate}->{$additive}->{$rolelist}->{"${prefix}${member}"} = 1;
                 }
             }
-            $result->{0}->{$l0}->{"${prefix}${member}"} = 1 if $l0;
-            $result->{1}->{$l1}->{"${prefix}${member}"} = 1 if $l1;
         }
     };
 
@@ -1738,20 +1822,39 @@ sub write_user_config {
 
             my $rolelist_members = {};
 
-            $collect_rolelist_members->($d->{'groups'}, $rolelist_members, '@');
+            $collect_rolelist_members->(
+                $d->{'groups'},
+                $d->{'additive_groups'} // {},
+                $rolelist_members,
+                '@',
+            );
 
             # no need to save 'root@pam', it is always 'Administrator'
-            $collect_rolelist_members->($d->{'users'}, $rolelist_members, '', 'root@pam');
+            $collect_rolelist_members->(
+                $d->{'users'},
+                $d->{'additive_users'} // {},
+                $rolelist_members,
+                '',
+                'root@pam',
+            );
 
-            $collect_rolelist_members->($d->{'tokens'}, $rolelist_members, '');
+            $collect_rolelist_members->(
+                $d->{'tokens'},
+                $d->{'additive_tokens'} // {},
+                $rolelist_members,
+                '',
+            );
 
             foreach my $propagate (0, 1) {
-                my $filtered = $rolelist_members->{$propagate};
-                foreach my $rolelist (sort keys %$filtered) {
-                    my $uglist = join(',', sort keys %{ $filtered->{$rolelist} });
-                    $data .= "acl:$propagate:$path:$uglist:$rolelist:\n";
+                foreach my $additive (0, 1) {
+                    my $filtered = $rolelist_members->{$propagate}->{$additive};
+                    next if !$filtered;
+                    foreach my $rolelist (sort keys %$filtered) {
+                        my $uglist = join(',', sort keys %{ $filtered->{$rolelist} });
+                        my $opts = $additive ? '+:' : '';
+                        $data .= "acl:$propagate:$path:$uglist:$rolelist:$opts\n";
+                    }
                 }
-
             }
         },
     );
@@ -1815,6 +1918,38 @@ sub roles {
     }
 
     my $roles = {};
+    # Roles carried forward from additive entries along the walk. Earliest writer wins, so a
+    # closer additive entry does not mask a farther one. This is independent of tier precedence:
+    # a carried role may come from any tier, even one whose entry did not win its level.
+    my $carried = {};
+
+    # Harvest a tier's additive roles into $carried. They carry propagate=1 by parser invariant
+    # (additive requires propagate); test truthiness, not definedness, so any in-memory drift to
+    # propagate=0 is filtered out and cannot carry to deeper paths.
+    my $harvest_additive = sub {
+        my ($ri, $additive_map) = @_;
+        return if !$ri || !$additive_map;
+        for my $role (keys %$additive_map) {
+            next if defined($carried->{$role});
+            next if !$ri->{$role};
+            $carried->{$role} = $ri->{$role};
+        }
+    };
+
+    # Collect a tier's roles that apply at this level, dropping non-propagating ones unless
+    # $final. $final is passed in, not closed over, because it is rebound each loop iteration.
+    my $collect_from_entry = sub {
+        my ($ri, $final) = @_;
+
+        my $new;
+        for my $role (keys %$ri) {
+            my $propagate = $ri->{$role};
+            next if !$final && !$propagate;
+            $new = {} if !$new;
+            $new->{$role} = $propagate;
+        }
+        return $new;
+    };
 
     my $split = [split("/", $path)];
     if ($path eq '/') {
@@ -1822,7 +1957,6 @@ sub roles {
     }
 
     my $acl = $cfg->{acl_root};
-    my $i = 0;
 
     while (@$split) {
         my $p = shift @$split;
@@ -1831,66 +1965,63 @@ sub roles {
             $acl = $acl->{children}->{$p};
         }
 
-        #print "CHECKACL $path $p\n";
-        #print "ACL $path = " . Dumper ($acl);
+        # Harvest every tier's additive roles before the precedence check below picks $roles, so
+        # a same-level higher-tier override cannot erase the carry of a lower-tier additive entry.
+        $harvest_additive->($acl->{tokens}->{$user}, ($acl->{additive_tokens} // {})->{$user});
+        $harvest_additive->($acl->{users}->{$user}, ($acl->{additive_users} // {})->{$user});
+        for my $g (keys %{ $acl->{groups} }) {
+            next if !$cfg->{groups}->{$g}->{users}->{$user};
+            $harvest_additive->($acl->{groups}->{$g}, ($acl->{additive_groups} // {})->{$g});
+        }
+
+        # Tier precedence (token > user > group): the first matching tier replaces $roles for this
+        # level, a deeper level may replace it again. The carry harvested above is unaffected.
         if (my $ri = $acl->{tokens}->{$user}) {
-            my $new;
-            foreach my $role (keys %$ri) {
-                my $propagate = $ri->{$role};
-                if ($final || $propagate) {
-                    #print "APPLY ROLE $p $user $role\n";
-                    $new = {} if !$new;
-                    $new->{$role} = $propagate;
-                }
-            }
+            my $new = $collect_from_entry->($ri, $final);
             if ($new) {
-                $roles = $new; # overwrite previous settings
+                $roles = $new;
                 next;
             }
         }
 
         if (my $ri = $acl->{users}->{$user}) {
-            my $new;
-            foreach my $role (keys %$ri) {
-                my $propagate = $ri->{$role};
-                if ($final || $propagate) {
-                    #print "APPLY ROLE $p $user $role\n";
-                    $new = {} if !$new;
-                    $new->{$role} = $propagate;
-                }
-            }
+            my $new = $collect_from_entry->($ri, $final);
             if ($new) {
-                $roles = $new; # overwrite previous settings
+                $roles = $new;
                 next; # user privs always override group privs
             }
         }
 
         my $new;
-        foreach my $g (keys %{ $acl->{groups} }) {
+        for my $g (keys %{ $acl->{groups} }) {
             next if !$cfg->{groups}->{$g}->{users}->{$user};
-            if (my $ri = $acl->{groups}->{$g}) {
-                foreach my $role (keys %$ri) {
-                    my $propagate = $ri->{$role};
-                    if ($final || $propagate) {
-                        #print "APPLY ROLE $p \@$g $role\n";
-                        $new = {} if !$new;
-                        $new->{$role} = $propagate;
-                    }
-                }
+            my $ri = $acl->{groups}->{$g};
+            next if !$ri;
+            my $g_new = $collect_from_entry->($ri, $final);
+            next if !$g_new;
+            $new = {} if !$new;
+            for my $role (keys %$g_new) {
+                $new->{$role} = $g_new->{$role};
             }
         }
         if ($new) {
-            $roles = $new; # overwrite previous settings
+            $roles = $new;
             next;
         }
     }
 
+    # NoAccess from an additive ancestor blocks everything below, even a deeper non-additive grant.
+    if (defined($carried->{NoAccess})) {
+        return { 'NoAccess' => $carried->{NoAccess} };
+    }
+
     return { 'NoAccess' => $roles->{NoAccess} } if defined($roles->{NoAccess});
-    #return () if defined ($roles->{NoAccess});
 
-    #print "permission $user $path = " . Dumper ($roles);
-
-    #print "roles $user $path = " . join (',', @ra) . "\n";
+    # Merge carried roles in; on a name clash the held $roles win, as the deeper ACL is more
+    # specific.
+    for my $role (keys %$carried) {
+        $roles->{$role} = $carried->{$role} if !defined($roles->{$role});
+    }
 
     return $roles;
 }
diff --git a/src/test/Makefile b/src/test/Makefile
index 53f2da7..c41ee1d 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,5 +12,6 @@ check:
 	perl -I.. perm-test6.pl
 	perl -I.. perm-test7.pl
 	perl -I.. perm-test8.pl
+	perl -I.. perm-test9.pl
 	perl -I.. realm_sync_test.pl
 	perl -I.. api-tests.pl
diff --git a/src/test/parser_writer.pl b/src/test/parser_writer.pl
index ea2778e..451e786 100755
--- a/src/test/parser_writer.pl
+++ b/src/test/parser_writer.pl
@@ -377,6 +377,47 @@ my $default_cfg = {
             },
         },
     },
+    acl_additive_user => {
+        'path' => '/',
+        users => {
+            'test@pam' => {
+                'PVEVMAdmin' => 1,
+            },
+        },
+        additive_users => {
+            'test@pam' => {
+                'PVEVMAdmin' => 1,
+            },
+        },
+    },
+    acl_additive_group => {
+        'path' => '/storage',
+        groups => {
+            'testgroup' => {
+                'PVEDatastoreAdmin' => 1,
+            },
+        },
+        additive_groups => {
+            'testgroup' => {
+                'PVEDatastoreAdmin' => 1,
+            },
+        },
+    },
+    acl_additive_user_mixed => {
+        # same member at same path with one additive and one non-additive role
+        'path' => '/',
+        users => {
+            'test@pam' => {
+                'PVEAuditor' => 1,
+                'PVEVMAdmin' => 1,
+            },
+        },
+        additive_users => {
+            'test@pam' => {
+                'PVEVMAdmin' => 1,
+            },
+        },
+    },
 };
 
 $default_cfg->{'acl_complex_mixed_root'} = {
@@ -457,6 +498,11 @@ my $default_raw = {
         'acl_complex_mixed_2' => 'acl:1:/storage:@testgroup,test@pam:PVEDatastoreAdmin:',
         'acl_complex_mixed_3' => 'acl:1:/storage:@another,test2@pam:PVEDatastoreUser:',
         'acl_missing_role' => 'acl:1:/storage:test@pam:MissingRole:',
+        'acl_additive_user' => 'acl:1:/:test@pam:PVEVMAdmin:+:',
+        'acl_additive_group' => 'acl:1:/storage:@testgroup:PVEDatastoreAdmin:+:',
+        # canonical output: one line per (propagate, additive) bucket, sorted
+        'acl_additive_user_mixed_canonical' => 'acl:1:/:test@pam:PVEAuditor:' . "\n"
+            . 'acl:1:/:test@pam:PVEVMAdmin:+:',
     },
 };
 
@@ -931,6 +977,109 @@ my $tests = [
             . "\n\n\n\n\n"
             . $default_raw->{acl}->{'acl_simple_user'} . "\n",
     },
+    {
+        name => "acl_additive_user",
+        config => {
+            users => default_users_with([$default_cfg->{test_pam}]),
+            roles => default_roles(),
+            acl_root => default_acls_with([$default_cfg->{acl_additive_user}]),
+        },
+        raw => ""
+            . $default_raw->{users}->{'root@pam'} . "\n"
+            . $default_raw->{users}->{'test_pam'}
+            . "\n\n\n\n\n"
+            . $default_raw->{acl}->{'acl_additive_user'} . "\n",
+    },
+    {
+        name => "acl_additive_group",
+        config => {
+            users => default_users_with([$default_cfg->{test_pam_with_group}]),
+            groups => default_groups_with([$default_cfg->{test_group_single_member}]),
+            roles => default_roles(),
+            acl_root => default_acls_with([$default_cfg->{acl_additive_group}]),
+        },
+        raw => ""
+            . $default_raw->{users}->{'root@pam'} . "\n"
+            . $default_raw->{users}->{'test_pam'} . "\n\n"
+            . $default_raw->{groups}->{'test_group_single_member'}
+            . "\n\n\n\n"
+            . $default_raw->{acl}->{'acl_additive_group'} . "\n",
+    },
+    {
+        # Regression: two roles on the same (member, path), one additive and one not, must
+        # round-trip as two separate lines. A per-member (rather than per-role) additive flag
+        # would merge them into one '+:' line, silently promoting the non-additive role.
+        name => "acl_additive_mixed_roles_roundtrip",
+        config => {
+            users => default_users_with([$default_cfg->{test_pam}]),
+            roles => default_roles(),
+            acl_root => default_acls_with([$default_cfg->{acl_additive_user_mixed}]),
+        },
+        raw => ""
+            . $default_raw->{users}->{'root@pam'} . "\n"
+            . $default_raw->{users}->{'test_pam'}
+            . "\n\n\n\n\n"
+            . $default_raw->{acl}->{'acl_additive_user_mixed_canonical'} . "\n",
+    },
+    {
+        # Duplicate (member, path, role) lines with conflicting flags: last line wins and the
+        # parser warns. Here the second line clears the additive flag that the first line set.
+        name => "acl_additive_conflict_warns",
+        raw => ""
+            . $default_raw->{users}->{'root@pam'} . "\n"
+            . $default_raw->{users}->{'test_pam'}
+            . "\n\n\n\n\n"
+            . 'acl:1:/:test@pam:PVEVMAdmin:+:' . "\n"
+            . 'acl:1:/:test@pam:PVEVMAdmin:' . "\n",
+        expected_config => {
+            users => default_users_with([$default_cfg->{test_pam}]),
+            roles => default_roles(),
+            acl_root => default_acls_with(
+                [{
+                    'path' => '/',
+                    users => {
+                        'test@pam' => {
+                            'PVEVMAdmin' => 1,
+                        },
+                    },
+                }],
+            ),
+        },
+        expected_raw => ""
+            . $default_raw->{users}->{'root@pam'} . "\n"
+            . $default_raw->{users}->{'test_pam'}
+            . "\n\n\n\n\n"
+            . $default_raw->{acl}->{'acl_simple_user'} . "\n",
+    },
+    {
+        # Reject 'additive' without 'propagate': the flag is dropped with a warning and only the
+        # propagate=0 entry survives.
+        name => "acl_additive_requires_propagate",
+        raw => ""
+            . $default_raw->{users}->{'root@pam'} . "\n"
+            . $default_raw->{users}->{'test_pam'}
+            . "\n\n\n\n\n"
+            . 'acl:0:/:test@pam:PVEVMAdmin:+:' . "\n",
+        expected_config => {
+            users => default_users_with([$default_cfg->{test_pam}]),
+            roles => default_roles(),
+            acl_root => default_acls_with(
+                [{
+                    'path' => '/',
+                    users => {
+                        'test@pam' => {
+                            'PVEVMAdmin' => 0,
+                        },
+                    },
+                }],
+            ),
+        },
+        expected_raw => ""
+            . $default_raw->{users}->{'root@pam'} . "\n"
+            . $default_raw->{users}->{'test_pam'}
+            . "\n\n\n\n\n"
+            . 'acl:0:/:test@pam:PVEVMAdmin:' . "\n",
+    },
     {
         name => "acl_complex_mixed",
         config => {
diff --git a/src/test/perm-test9.pl b/src/test/perm-test9.pl
new file mode 100644
index 0000000..a63b719
--- /dev/null
+++ b/src/test/perm-test9.pl
@@ -0,0 +1,181 @@
+#!/usr/bin/perl
+
+use v5.36;
+
+use Test::More;
+
+use PVE::AccessControl;
+use PVE::RPCEnvironment;
+
+my $rpcenv = PVE::RPCEnvironment->init('cli');
+$rpcenv->init_request(userconfig => 'test9.cfg');
+
+sub check_roles($user, $path, $expected, $desc) {
+    my $roles = PVE::AccessControl::roles($rpcenv->{user_cfg}, $user, $path);
+    is(join(',', sort keys %$roles), $expected, "$desc - roles($user, $path)");
+}
+
+sub check_permissions($user, $path, $expected, $desc) {
+    my $perm = $rpcenv->permissions($user, $path);
+    is(join(',', sort keys %$perm), $expected, "$desc - perms($user, $path)");
+}
+
+# Each group is a Test::More subtest; each case lists a user plus expected role/perm sets per path.
+# The case desc plus user/path is emitted with every assertion to self-locate failures.
+my @groups = (
+    {
+        name => 'additive carry across a deeper replace',
+        cases => [
+            {
+                desc => 'User2 (ADMINS+OPS): RoleADMIN carried, merged with deeper RoleOPS',
+                user => 'User2@pve',
+                roles => { '/pool/top/sub' => 'RoleADMIN,RoleOPS' },
+                # exercises pool->vm projection and role->priv expansion
+                perms => { '/vms/200' => 'VM.Audit,VM.Console,VM.PowerMgmt' },
+            },
+            {
+                desc => 'User3 (OPS only): additive at top inert when not in @ADMINS',
+                user => 'User3@pve',
+                roles => { '/pool/top/sub' => 'RoleOPS' },
+            },
+        ],
+    },
+    {
+        name => 'multi-level walk preserves carry across replaces',
+        cases => [
+            {
+                desc => 'User2: carry survives replace at sub, propagates through deep',
+                user => 'User2@pve',
+                roles => { '/pool/top/sub/deep' => 'RoleADMIN,RoleOPS' },
+            },
+            {
+                desc => 'User4 (only deep ACL, no group): additive at unrelated ancestor inert',
+                user => 'User4@pve',
+                roles => { '/pool/top/sub/deep' => 'RoleCHILD' },
+            },
+        ],
+    },
+    {
+        name => 'additive carry survives a non-propagating deeper entry',
+        cases => [
+            {
+                desc => 'User2: carried RoleADMIN merges with a deeper propagate=0 RoleCHILD',
+                user => 'User2@pve',
+                roles => { '/pool/nonprop/mid' => 'RoleADMIN,RoleCHILD' },
+            },
+            {
+                desc => 'User2: propagate=0 RoleCHILD does not descend, carried RoleADMIN does',
+                user => 'User2@pve',
+                roles => { '/pool/nonprop/mid/leaf' => 'RoleADMIN' },
+            },
+        ],
+    },
+    {
+        name => 'baseline (no additive flag) keeps replace semantics',
+        cases => [
+            {
+                desc => 'User2: deeper @OPS replaces RoleADMIN, no carry without flag',
+                user => 'User2@pve',
+                roles => { '/pool/baseline/child' => 'RoleOPS' },
+                perms => { '/vms/500' => 'VM.Audit' },
+            },
+        ],
+    },
+    {
+        name => 'NoAccess on additive entry blocks deeper grants',
+        cases => [
+            {
+                desc => 'User1: additive NoAccess at parent overrides child RoleADMIN',
+                user => 'User1@pve',
+                roles => { '/pool/denied/child' => 'NoAccess' },
+                perms => { '/vms/700' => '' },
+            },
+        ],
+    },
+    {
+        name => 'deeper non-additive NoAccess overrides a carried additive grant',
+        cases => [
+            {
+                desc => 'User1: child NoAccess wins over the carried additive RoleADMIN',
+                user => 'User1@pve',
+                roles => { '/pool/denygrant/child' => 'NoAccess' },
+                perms => { '/vms/931' => '' },
+            },
+        ],
+    },
+    {
+        name => 'multi-group at same path: only the additive group carries',
+        cases => [
+            {
+                desc => 'MixUser (GI additive + GN non-additive): same-level union still works',
+                user => 'MixUser@pve',
+                roles => { '/pool/mixgroups' => 'RoleGI,RoleGN' },
+            },
+            {
+                desc => 'User2 (ADMINS non-additive + OPS additive): RoleADMIN must not leak',
+                user => 'User2@pve',
+                roles => { '/pool/multigrp/child' => 'RoleCHILD,RoleOPS' },
+            },
+        ],
+    },
+    {
+        name => 'additive is independent of same-level tier precedence',
+        cases => [
+            {
+                desc => 'User2 (group-tier additive + user-tier non-additive at same path)',
+                user => 'User2@pve',
+                roles => {
+                    '/pool/tieroverlap' => 'RoleADMIN,RoleCHILD',
+                    '/pool/tieroverlap/child' => 'RoleADMIN,RoleCHILD',
+                },
+            },
+        ],
+    },
+    {
+        name => 'privsep tokens do not inherit user-tier additive carry',
+        cases => [
+            {
+                desc => 'TokenUser user view: RoleADMIN carried past child @OPS replace',
+                user => 'TokenUser@pve',
+                roles => { '/pool/tokenpool/child' => 'RoleADMIN,RoleOPS' },
+            },
+            {
+                desc => 'privsep token: own ACL only, no carry from user-tier additive',
+                user => 'TokenUser@pve!privsep',
+                roles => { '/pool/tokenpool/child' => 'RoleOPS' },
+                perms => { '/vms/901' => 'VM.Audit' },
+            },
+            {
+                desc => 'full token: short-circuits to user roles incl. carried RoleADMIN',
+                user => 'TokenUser@pve!full',
+                roles => { '/pool/tokenpool/child' => 'RoleADMIN,RoleOPS' },
+                perms => { '/vms/901' => 'VM.Audit,VM.Console,VM.PowerMgmt' },
+            },
+            {
+                desc => 'privsep token: its own additive entry carries past a deeper replace',
+                user => 'TokenUser@pve!privsep',
+                roles => { '/pool/tokadd/child' => 'RoleADMIN,RoleOPS' },
+            },
+        ],
+    },
+);
+
+for my $group (@groups) {
+    subtest $group->{name} => sub {
+        for my $case ($group->{cases}->@*) {
+            my $desc = $case->{desc};
+            if (my $r = $case->{roles}) {
+                for my $path (sort keys %$r) {
+                    check_roles($case->{user}, $path, $r->{$path}, $desc);
+                }
+            }
+            if (my $p = $case->{perms}) {
+                for my $path (sort keys %$p) {
+                    check_permissions($case->{user}, $path, $p->{$path}, $desc);
+                }
+            }
+        }
+    };
+}
+
+done_testing();
diff --git a/src/test/test9.cfg b/src/test/test9.cfg
new file mode 100644
index 0000000..c7cce08
--- /dev/null
+++ b/src/test/test9.cfg
@@ -0,0 +1,86 @@
+user:User1@pve:1:
+user:User2@pve:1:
+user:User3@pve:1:
+user:User4@pve:1:
+user:MixUser@pve:1:
+user:TokenUser@pve:1:
+
+token:TokenUser@pve!privsep:0:1::
+token:TokenUser@pve!full:0:0::
+
+group:ADMINS:User1@pve,User2@pve:
+group:OPS:User2@pve,User3@pve:
+group:GI:MixUser@pve:
+group:GN:MixUser@pve:
+
+role:RoleADMIN:VM.PowerMgmt,VM.Console:
+role:RoleOPS:VM.Audit:
+role:RoleCHILD:VM.Migrate:
+role:RoleGI:VM.Snapshot:
+role:RoleGN:VM.Backup:
+
+pool:top:top pool:100::
+pool:top/sub::200,201:store1:
+pool:top/sub/deep::300::
+
+pool:baseline::400::
+pool:baseline/child::500:store2:
+
+pool:denied:denied pool:600::
+pool:denied/child::700::
+
+pool:mixgroups::800::
+pool:mixgroups/child::801::
+
+pool:multigrp::850::
+pool:multigrp/child::851::
+
+pool:tieroverlap::870::
+pool:tieroverlap/child::871::
+
+pool:tokenpool::900::
+pool:tokenpool/child::901::
+
+pool:nonprop::920::
+pool:nonprop/mid::921::
+pool:nonprop/mid/leaf::922::
+
+pool:denygrant::930::
+pool:denygrant/child::931::
+
+pool:tokadd::940::
+pool:tokadd/child::941::
+
+acl:1:/pool/top:@ADMINS:RoleADMIN:+:
+acl:1:/pool/top/sub:@OPS:RoleOPS:
+acl:1:/pool/top/sub/deep:User4@pve:RoleCHILD:
+
+acl:1:/pool/baseline:@ADMINS:RoleADMIN:
+acl:1:/pool/baseline/child:@OPS:RoleOPS:
+
+acl:1:/pool/denied:User1@pve:NoAccess:+:
+acl:1:/pool/denied/child:User1@pve:RoleADMIN:
+
+acl:1:/pool/mixgroups:@GI:RoleGI:+:
+acl:1:/pool/mixgroups:@GN:RoleGN:
+acl:1:/pool/mixgroups/child:@OPS:RoleOPS:
+
+acl:1:/pool/multigrp:@ADMINS:RoleADMIN:
+acl:1:/pool/multigrp:@OPS:RoleOPS:+:
+acl:1:/pool/multigrp/child:User2@pve:RoleCHILD:
+
+acl:1:/pool/tieroverlap:@ADMINS:RoleADMIN:+:
+acl:1:/pool/tieroverlap:User2@pve:RoleCHILD:
+
+acl:1:/pool/tokenpool:TokenUser@pve:RoleADMIN:+:
+acl:1:/pool/tokenpool/child:TokenUser@pve:RoleOPS:
+acl:1:/pool/tokenpool/child:TokenUser@pve!privsep:RoleOPS:
+
+acl:1:/pool/nonprop:@ADMINS:RoleADMIN:+:
+acl:0:/pool/nonprop/mid:User2@pve:RoleCHILD:
+
+acl:1:/pool/denygrant:User1@pve:RoleADMIN:+:
+acl:1:/pool/denygrant/child:User1@pve:NoAccess:
+
+acl:1:/pool/tokadd:TokenUser@pve!privsep:RoleADMIN:+:
+acl:1:/pool/tokadd/child:TokenUser@pve!privsep:RoleOPS:
-- 
2.47.3





  reply	other threads:[~2026-07-15 23:22 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-15 23:19 [RFC PATCH 0/4] add 'additive' flag to ACL entries Thomas Lamprecht
2026-07-15 23:19 ` Thomas Lamprecht [this message]
2026-07-15 23:20 ` [RFC PATCH access-control 2/4] api: acl: expose 'additive' flag via GET and PUT Thomas Lamprecht
2026-07-15 23:20 ` [RFC PATCH manager 3/4] ui: acl: expose the 'additive' ACL flag Thomas Lamprecht
2026-07-15 23:20 ` [RFC PATCH docs 4/4] pveum: document 'additive' ACL option Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260715232121.1009607-2-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal