* [RFC PATCH 0/4] add 'additive' flag to ACL entries
@ 2026-07-15 23:19 Thomas Lamprecht
2026-07-15 23:19 ` [RFC PATCH access-control 1/4] acl: support 'additive' option on " Thomas Lamprecht
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2026-07-15 23:19 UTC (permalink / raw)
To: pve-devel
By default a more specific ACL entry on a deeper path fully replaces the
roles inherited from ancestor paths. That works well for scoped
delegation, but it is awkward when a role should apply to a whole
subtree. Take an auditor group that needs read access everywhere: today
that role has to be repeated on every deeper entry, and it quietly stops
applying as soon as someone adds a more specific ACL below.
I run into this frequently on some of our own infra especially if users
are part of multiple groups.
So, to make this a bit easier this series adds an optional per-entry
'additive' flag. An additive entry's roles are carried down the walk and
merged with more specific entries below instead of being replaced;
NoAccess still dominates. On disk it is a compact '+' option field on
the acl: line, exposed via the API and CLI as a boolean 'additive'.
See the docs patch - especially its Notes section - for some more
details.
Sending as RFC mainly for feedback on the overall semantics and also
happy to bikeshed the name and the on-disk '+' encoding (an extensible
option field, so future flags need no new positional column).
Basic docs and UI integration included to make it a bit more complete
and easier to test.
pve-access-control:
acl: support 'additive' option on ACL entries
api: acl: expose 'additive' flag via GET and PUT
src/PVE/API2/ACL.pm | 75 ++++++++++--
src/PVE/AccessControl.pm | 247 +++++++++++++++++++++++++++++---------
src/test/Makefile | 2 +
src/test/api-acl-test.pl | 223 ++++++++++++++++++++++++++++++++++
src/test/parser_writer.pl | 149 +++++++++++++++++++++++
src/test/perm-test9.pl | 181 ++++++++++++++++++++++++++++
src/test/test9.cfg | 86 +++++++++++++
7 files changed, 898 insertions(+), 65 deletions(-)
pve-manager:
ui: acl: expose the 'additive' ACL flag
www/manager6/dc/ACLView.js | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
pve-docs:
pveum: document 'additive' ACL option
pveum.adoc | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
base-commit: dcda7019c90c550995e784c165c9786d5cef4ed3
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH access-control 1/4] acl: support 'additive' option on ACL entries
2026-07-15 23:19 [RFC PATCH 0/4] add 'additive' flag to ACL entries Thomas Lamprecht
@ 2026-07-15 23:19 ` Thomas Lamprecht
2026-07-15 23:20 ` [RFC PATCH access-control 2/4] api: acl: expose 'additive' flag via GET and PUT Thomas Lamprecht
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2026-07-15 23:19 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH access-control 2/4] api: acl: expose 'additive' flag via GET and PUT
2026-07-15 23:19 [RFC PATCH 0/4] add 'additive' flag to ACL entries Thomas Lamprecht
2026-07-15 23:19 ` [RFC PATCH access-control 1/4] acl: support 'additive' option on " Thomas Lamprecht
@ 2026-07-15 23:20 ` 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
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2026-07-15 23:20 UTC (permalink / raw)
To: pve-devel
Expose the new 'additive' flag through the REST API so tools like the
web UI and pveum can read and set it. GET returns it per entry next
to 'propagate', and PUT gains a matching parameter.
Treat the PUT parameter as tri-state: leaving it out keeps the stored
flag unchanged. That way a client that does not know the option, or
that just re-saves an entry to change something else, cannot wipe an
additive flag that was configured earlier. Passing 'additive=0'
clears it.
Reject 'additive' without 'propagate', same as the parser does. Skip
that check when deleting, where both flags are ignored anyway, so a
cleanup call can reuse the same parameters as the matching add call.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
src/PVE/API2/ACL.pm | 75 +++++++++++--
src/test/Makefile | 1 +
src/test/api-acl-test.pl | 223 +++++++++++++++++++++++++++++++++++++++
3 files changed, 292 insertions(+), 7 deletions(-)
create mode 100644 src/test/api-acl-test.pl
diff --git a/src/PVE/API2/ACL.pm b/src/PVE/API2/ACL.pm
index c57dd38..10116f9 100644
--- a/src/PVE/API2/ACL.pm
+++ b/src/PVE/API2/ACL.pm
@@ -17,12 +17,27 @@ use base qw(PVE::RESTHandler);
register_standard_option(
'acl-propagate',
{
- description => "Allow to propagate (inherit) permissions.",
+ description => "Allow to propagate (inherit) permissions. See also 'additive' to merge"
+ . " instead of replace at deeper paths.",
type => 'boolean',
optional => 1,
default => 1,
},
);
+my $acl_additive_desc =
+ "Make permissions accumulate instead of being replaced at deeper paths: this entry's roles"
+ . " are merged with roles granted by more specific ACL entries below, rather than being"
+ . " overridden by them. A deeper entry can still revoke a carried role via 'NoAccess', but"
+ . " not by granting a narrower one. Requires 'propagate'.";
+register_standard_option(
+ 'acl-additive',
+ {
+ description => $acl_additive_desc,
+ type => 'boolean',
+ optional => 1,
+ default => 0,
+ },
+);
register_standard_option(
'acl-path',
{
@@ -52,6 +67,7 @@ __PACKAGE__->register_method({
additionalProperties => 0,
properties => {
propagate => get_standard_option('acl-propagate'),
+ additive => get_standard_option('acl-additive'),
path => get_standard_option('acl-path'),
type => { type => 'string', enum => ['user', 'group', 'token'] },
ugid => { type => 'string' },
@@ -83,7 +99,9 @@ __PACKAGE__->register_method({
my $d = $node->{"${type}s"};
next if !$d;
next if !($audit || $rpcenv->check_perm_modify($authuser, $path, 1));
+ my $additive_members = $node->{"additive_${type}s"} // {};
foreach my $id (keys %$d) {
+ my $additive_roles = $additive_members->{$id} // {};
foreach my $role (keys %{ $d->{$id} }) {
my $propagate = $d->{$id}->{$role};
push @$res,
@@ -93,6 +111,7 @@ __PACKAGE__->register_method({
ugid => $id,
roleid => $role,
propagate => $propagate,
+ additive => $additive_roles->{$role} ? 1 : 0,
};
}
}
@@ -117,6 +136,14 @@ __PACKAGE__->register_method({
additionalProperties => 0,
properties => {
propagate => get_standard_option('acl-propagate'),
+ additive => get_standard_option(
+ 'acl-additive',
+ {
+ description => $acl_additive_desc
+ . " Omit on update to keep the stored value; pass explicitly to set or"
+ . " clear.",
+ },
+ ),
path => get_standard_option('acl-path'),
users => {
description => "List of users.",
@@ -166,6 +193,28 @@ __PACKAGE__->register_method({
raise_param_exc({ path => "invalid ACL path '$param->{path}'" });
}
+ my $propagate = 1;
+ if (defined($param->{propagate})) {
+ $propagate = $param->{propagate} ? 1 : 0;
+ }
+
+ # Tri-state: undef means "leave the stored flag alone", so a caller that does not send the
+ # parameter (older clients, UIs that predate the option) cannot silently clear a configured
+ # additive flag on an unrelated edit.
+ my $additive;
+ if (defined($param->{additive})) {
+ $additive = $param->{additive} ? 1 : 0;
+ }
+
+ # 'additive' on a non-propagating entry has nothing to merge down, so reject the combo up
+ # front. Delete ignores both flags, skip the check there to not block cleanup scripts.
+ raise_param_exc({ additive => "'additive' requires 'propagate'" })
+ if $additive && !$propagate && !$param->{delete};
+
+ # Also clear it when 'propagate' flips to 0 while 'additive' is omitted, so a GET reading
+ # this config before the writer strips it never sees the rejected combo.
+ $additive = 0 if !$propagate && !$param->{delete};
+
PVE::AccessControl::lock_user_config(
sub {
my $cfg = cfs_read_file("user.cfg");
@@ -174,12 +223,6 @@ __PACKAGE__->register_method({
my $authuser = $rpcenv->get_user();
my $auth_user_privs = $rpcenv->permissions($authuser, $path);
- my $propagate = 1;
-
- if (defined($param->{propagate})) {
- $propagate = $param->{propagate} ? 1 : 0;
- }
-
my $node = PVE::AccessControl::find_acl_tree_node($cfg->{acl_root}, $path);
foreach my $role (split_list($param->{roles})) {
@@ -228,8 +271,14 @@ __PACKAGE__->register_method({
if ($param->{delete}) {
delete($node->{groups}->{$group}->{$role});
+ PVE::AccessControl::update_acl_additive(
+ $node, 'additive_groups', $group, $role, 0,
+ );
} else {
$node->{groups}->{$group}->{$role} = $propagate;
+ PVE::AccessControl::update_acl_additive(
+ $node, 'additive_groups', $group, $role, $additive,
+ ) if defined($additive);
}
}
@@ -241,8 +290,14 @@ __PACKAGE__->register_method({
if ($param->{delete}) {
delete($node->{users}->{$username}->{$role});
+ PVE::AccessControl::update_acl_additive(
+ $node, 'additive_users', $username, $role, 0,
+ );
} else {
$node->{users}->{$username}->{$role} = $propagate;
+ PVE::AccessControl::update_acl_additive(
+ $node, 'additive_users', $username, $role, $additive,
+ ) if defined($additive);
}
}
@@ -252,8 +307,14 @@ __PACKAGE__->register_method({
if ($param->{delete}) {
delete $node->{tokens}->{$tokenid}->{$role};
+ PVE::AccessControl::update_acl_additive(
+ $node, 'additive_tokens', $tokenid, $role, 0,
+ );
} else {
$node->{tokens}->{$tokenid}->{$role} = $propagate;
+ PVE::AccessControl::update_acl_additive(
+ $node, 'additive_tokens', $tokenid, $role, $additive,
+ ) if defined($additive);
}
}
}
diff --git a/src/test/Makefile b/src/test/Makefile
index c41ee1d..db1e0c4 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -15,3 +15,4 @@ check:
perl -I.. perm-test9.pl
perl -I.. realm_sync_test.pl
perl -I.. api-tests.pl
+ perl -I.. api-acl-test.pl
diff --git a/src/test/api-acl-test.pl b/src/test/api-acl-test.pl
new file mode 100644
index 0000000..9aaabc7
--- /dev/null
+++ b/src/test/api-acl-test.pl
@@ -0,0 +1,223 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use lib qw(..);
+
+use Test::More;
+use Test::MockModule;
+
+use PVE::AccessControl;
+use PVE::RPCEnvironment;
+use PVE::API2::ACL;
+
+# Fake the cluster file layer so we can drive the PUT handler against an in-process config
+# instead of /etc/pve.
+my $fake_cfg;
+my $cluster_module = Test::MockModule->new('PVE::Cluster');
+$cluster_module->noop('cfs_update');
+$cluster_module->mock('cfs_file_version', sub { return time(); });
+
+# AccessControl and API2::ACL import the cfs_* functions directly, so mocking the source package
+# does not affect their imported aliases. Install replacements in every namespace holding a ref.
+{
+ my $read = sub { return $fake_cfg; };
+ my $write = sub { $fake_cfg = $_[1]; };
+ my $lock = sub { my (undef, undef, $code) = @_; $code->(); };
+ no strict 'refs'; ## no critic
+ no warnings 'redefine'; ## no critic
+ for my $pkg ('PVE::Cluster', 'PVE::AccessControl', 'PVE::API2::ACL') {
+ *{"${pkg}::cfs_read_file"} = $read;
+ *{"${pkg}::cfs_write_file"} = $write;
+ *{"${pkg}::cfs_lock_file"} = $lock;
+ }
+}
+
+my $rpcenv = PVE::RPCEnvironment->init('cli');
+
+# Seed $fake_cfg with a minimal starting config before init_request reads it.
+$fake_cfg = {};
+PVE::AccessControl::userconfig_force_defaults($fake_cfg);
+$fake_cfg->{users}->{'root@pam'} = { enable => 1, expire => 0 };
+
+$rpcenv->init_request();
+$rpcenv->set_user('root@pam');
+
+# Mock permission checks since root@pam bypasses them anyway; keep it explicit so the test does
+# not depend on subtle rpcenv state.
+my $rpcenv_module = Test::MockModule->new('PVE::RPCEnvironment');
+$rpcenv_module->mock(
+ 'permissions', sub { return { 'Permissions.Modify' => 1 }; },
+);
+
+sub fresh_cfg {
+ my $cfg = {};
+ PVE::AccessControl::userconfig_force_defaults($cfg);
+ $cfg->{users}->{'root@pam'} = { enable => 1, expire => 0 };
+ $cfg->{users}->{'alice@pve'} = { enable => 1, expire => 0 };
+ return $cfg;
+}
+
+my ($handler, $info) = PVE::API2::ACL->find_handler('PUT', '');
+
+sub put_acl {
+ my ($p) = @_;
+ eval { $handler->handle($info, $p); };
+ return $@;
+}
+
+# PUT without 'additive' must preserve the existing flag
+
+$fake_cfg = fresh_cfg();
+put_acl({
+ path => '/vms/100',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 1,
+ additive => 1,
+});
+is(
+ $fake_cfg->{acl_root}->{children}->{vms}->{children}->{100}->{additive_users}->{'alice@pve'}
+ ->{'PVEAuditor'},
+ 1,
+ 'additive=1 stored',
+);
+
+# Unaware caller edits the same entry without passing additive; flag must
+# survive.
+put_acl({
+ path => '/vms/100',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 1,
+});
+is(
+ $fake_cfg->{acl_root}->{children}->{vms}->{children}->{100}->{additive_users}->{'alice@pve'}
+ ->{'PVEAuditor'},
+ 1,
+ 'additive preserved when caller omits the parameter',
+);
+
+# Explicit additive=0 clears the flag.
+put_acl({
+ path => '/vms/100',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 1,
+ additive => 0,
+});
+is(
+ exists($fake_cfg->{acl_root}->{children}->{vms}->{children}->{100}->{additive_users}),
+ '',
+ 'additive=0 clears the flag (and scaffolding)',
+);
+
+# Delete + additive=1 must not error, even on non-propagating call
+
+$fake_cfg = fresh_cfg();
+put_acl({
+ path => '/vms/200',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 1,
+ additive => 1,
+});
+is(
+ put_acl({
+ path => '/vms/200',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 0,
+ additive => 1,
+ delete => 1,
+ }),
+ '',
+ 'delete accepts additive=1 with propagate=0',
+);
+is(
+ $fake_cfg->{acl_root}->{children}->{vms}->{children}->{200}->{users}->{'alice@pve'}
+ ->{'PVEAuditor'},
+ undef,
+ 'role was actually removed',
+);
+
+# Non-delete with additive=1 + propagate=0 still errors
+
+$fake_cfg = fresh_cfg();
+my $err = put_acl({
+ path => '/vms/300',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 0,
+ additive => 1,
+});
+like(
+ "$err", qr/'additive' requires 'propagate'/, 'non-delete rejects additive + !propagate',
+);
+
+# roles() must not carry a propagate=0 role even if its additive flag is set. This drift state is
+# not reachable through the API (PUT clears it, see next test) or the parser (it strips the flag),
+# so construct it directly here to lock in the roles() guard regardless of how it might arise.
+
+$fake_cfg = fresh_cfg();
+my $node = PVE::AccessControl::find_acl_tree_node($fake_cfg->{acl_root}, '/vms/400');
+$node->{users}->{'alice@pve'}->{'PVEAuditor'} = 0; # propagate=0
+$node->{additive_users}->{'alice@pve'}->{'PVEAuditor'} = 1;
+my $drift_roles = PVE::AccessControl::roles($fake_cfg, 'alice@pve', '/vms/400/sub');
+is_deeply(
+ $drift_roles,
+ {},
+ 'harvest_additive: propagate=0 in-memory drift does not carry to deeper paths',
+);
+
+# Flipping propagate to 0 clears a stale additive flag
+# A non-propagating entry cannot carry roles down, so setting propagate=0
+# must normalize the now-invalid 'additive' flag in-memory, even when the
+# caller omits 'additive' on that update.
+
+$fake_cfg = fresh_cfg();
+put_acl({
+ path => '/vms/500',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 1,
+ additive => 1,
+});
+put_acl({
+ path => '/vms/500',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 0,
+});
+is(
+ exists($fake_cfg->{acl_root}->{children}->{vms}->{children}->{500}->{additive_users}),
+ '',
+ 'propagate=0 clears the additive flag even when additive is omitted',
+);
+
+# GET surfaces the additive flag per entry
+
+$fake_cfg = fresh_cfg();
+put_acl({
+ path => '/vms/600',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 1,
+ additive => 1,
+});
+put_acl({
+ path => '/vms/601',
+ users => 'alice@pve',
+ roles => 'PVEAuditor',
+ propagate => 1,
+});
+$rpcenv->{user_cfg} = $fake_cfg;
+my ($get_handler, $get_info) = PVE::API2::ACL->find_handler('GET', '');
+my $acls = $get_handler->handle($get_info, {});
+my %additive_by_path =
+ map { $_->{path} => $_->{additive} } grep { $_->{ugid} eq 'alice@pve' } @$acls;
+is($additive_by_path{'/vms/600'}, 1, 'GET reports additive=1 for the additive entry');
+is($additive_by_path{'/vms/601'}, 0, 'GET reports additive=0 for the plain entry');
+
+done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH manager 3/4] ui: acl: expose the 'additive' ACL flag
2026-07-15 23:19 [RFC PATCH 0/4] add 'additive' flag to ACL entries Thomas Lamprecht
2026-07-15 23:19 ` [RFC PATCH access-control 1/4] acl: support 'additive' option on " Thomas Lamprecht
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 ` Thomas Lamprecht
2026-07-15 23:20 ` [RFC PATCH docs 4/4] pveum: document 'additive' ACL option Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2026-07-15 23:20 UTC (permalink / raw)
To: pve-devel
Add an Additive checkbox to the add-permission dialog and a matching
column to the ACL grid so the flag can be set and seen from the web UI.
Additive requires propagate; tie the checkbox to Propagate (disabled
and cleared when Propagate is off) to mirror the API, which rejects
that combination.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
www/manager6/dc/ACLView.js | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/www/manager6/dc/ACLView.js b/www/manager6/dc/ACLView.js
index 7cc260e02..5091f7b53 100644
--- a/www/manager6/dc/ACLView.js
+++ b/www/manager6/dc/ACLView.js
@@ -61,6 +61,22 @@ Ext.define('PVE.dc.ACLAdd', {
checked: true,
uncheckedValue: 0,
fieldLabel: gettext('Propagate'),
+ listeners: {
+ // 'additive' requires 'propagate', so keep the checkbox in sync.
+ change: function (checkbox, value) {
+ let additive = checkbox.up('window').down('[name=additive]');
+ additive.setDisabled(!value);
+ if (!value) {
+ additive.setValue(false);
+ }
+ },
+ },
+ });
+ items.push({
+ xtype: 'proxmoxcheckbox',
+ name: 'additive',
+ uncheckedValue: 0,
+ fieldLabel: gettext('Additive'),
});
}
@@ -152,6 +168,12 @@ Ext.define(
sortable: true,
dataIndex: 'propagate',
});
+ columns.push({
+ header: gettext('Additive'),
+ width: 80,
+ sortable: true,
+ dataIndex: 'additive',
+ });
}
let sm = Ext.create('Ext.selection.RowModel', {});
@@ -264,6 +286,10 @@ Ext.define(
name: 'propagate',
type: 'boolean',
},
+ {
+ name: 'additive',
+ type: 'boolean',
+ },
],
});
},
base-commit: 20242970da7fbcef28aa6844d5d889319cb532ef
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH docs 4/4] pveum: document 'additive' ACL option
2026-07-15 23:19 [RFC PATCH 0/4] add 'additive' flag to ACL entries Thomas Lamprecht
` (2 preceding siblings ...)
2026-07-15 23:20 ` [RFC PATCH manager 3/4] ui: acl: expose the 'additive' ACL flag Thomas Lamprecht
@ 2026-07-15 23:20 ` Thomas Lamprecht
3 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2026-07-15 23:20 UTC (permalink / raw)
To: pve-devel
The additive flag overrides the default "deeper replaces upper" rule
for individual ACL entries. Describe the semantics, the interaction
with NoAccess, and the requirement for 'propagate' so admins can
choose between replace-based scoped delegation and accumulate-based
broad permissions.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
pveum.adoc | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/pveum.adoc b/pveum.adoc
index d089cb6..b8781e1 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -983,6 +983,54 @@ set by default). We use the following inheritance rules:
Additionally, privilege separated tokens can never have permissions on any
given path that their associated user does not have.
+[[pveum_additive_permissions]]
+Additive Permissions
+++++++++++++++++++++
+
+By default, an ACL entry on a deeper path fully replaces the roles inherited
+from ancestor paths. Sometimes you want a broader set of permissions granted
+at a high level to remain in effect even where a more specific ACL entry is
+set below. A typical case is auditors that should keep read access across
+the entire tree regardless of scoped delegations further down.
+
+Setting the `additive` flag on an ACL entry makes its roles accumulate rather
+than be replaced: the entry's roles are merged with roles granted by more
+specific ACL entries below, instead of being overridden by them.
+
+For example, granting an `auditors` group `PVEAuditor` additively at the
+root path:
+
+----
+# pveum acl modify / --groups auditors --roles PVEAuditor --propagate 1 --additive 1
+----
+
+then assigning a regular non-additive role to a user on a specific VM:
+
+----
+# pveum acl modify /vms/100 --users alice@pve --roles PVEVMAdmin
+----
+
+leaves `alice@pve` (assuming they are a member of the `auditors` group) with
+both `PVEAuditor` (carried from the additive ancestor entry) and
+`PVEVMAdmin` (from the direct ACL) when accessing VM 100.
+
+Notes:
+
+* `additive` requires `propagate` to be set; an entry that does not cascade
+ down has nothing to merge into deeper paths.
+* `additive` is independent of tier precedence: even when a higher-tier ACL
+ entry (`token` over `user` over `group`) wins the role set at a given
+ level, additive roles from any tier at that level still carry forward.
+* `NoAccess` on an additive entry blocks all access at deeper paths, even
+ against roles granted by more specific ACL entries below. Conversely, a
+ non-additive `NoAccess` set at a deeper path cancels carried additive
+ roles at and below that path, matching the rule that `NoAccess` cancels
+ all other roles on a given path.
+* Privilege separated API tokens are evaluated against their own ACL tree.
+ Additive entries on the underlying user do not leak into the token's
+ effective roles; its effective privileges remain the intersection of its
+ own ACL grants and the user's privileges.
+
[[pveum_pools]]
Pools
~~~~~
base-commit: f2632cea11fc1000ddff53249527ca2493372a13
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-15 23:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 23:19 [RFC PATCH 0/4] add 'additive' flag to ACL entries Thomas Lamprecht
2026-07-15 23:19 ` [RFC PATCH access-control 1/4] acl: support 'additive' option on " Thomas Lamprecht
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
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.