From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 2692A1FF0DF for ; Thu, 05 Nov 2026 11:38:01 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 618AC21667; Thu, 05 Nov 2026 11:37:44 +0100 (CET) From: Fiona Ebner To: pve-devel@lists.proxmox.com Subject: [PATCH storage v2 1/2] api: prune backups: align permission check for dry run and active run Date: Thu, 5 Nov 2026 11:35:43 +0100 Message-ID: <20261105103736.52417-2-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261105103736.52417-1-f.ebner@proxmox.com> References: <20261105103736.52417-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793875059765 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.296 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ISWRTV4J6AFHDS4LYUYMPELCU4LXAX4U X-Message-ID-Hash: ISWRTV4J6AFHDS4LYUYMPELCU4LXAX4U X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The API endpoint for listing storage content filters volumes with check_volume_access(). The dry run for prune backups should not return volumes that the content listing does not show. While check_volume_access() could be used there as well, access to a backup volume already requires 'Datastore.AllocateSpace' for the storage and 'VM.Backup' for the guest, or 'Datastore.Allocate' for the storage. These are the same permissions required for the actual pruning. The dry run should show what an actual pruning invocation with the same parameters would do. Using check_volume_access() would, for invocations without an explicit guest ID, show what a storage-wide prune would do for all guest groups the user has access to. This would be in contrast to the actual pruning invocation, which requires 'Datastore.Allocate' for an invocation without an explicit guest ID, so there would be a mismatch. Together, these points suggest aligning the permission check between the endpoint for the dry run and the endpoint for the active run. Signed-off-by: Fiona Ebner --- src/PVE/API2/Storage/PruneBackups.pm | 34 ++++++++++++++++++---------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/PVE/API2/Storage/PruneBackups.pm b/src/PVE/API2/Storage/PruneBackups.pm index 2920c899..7863c36e 100644 --- a/src/PVE/API2/Storage/PruneBackups.pm +++ b/src/PVE/API2/Storage/PruneBackups.pm @@ -12,6 +12,19 @@ use PVE::Tools qw(extract_param); use base qw(PVE::RESTHandler); +my sub assert_prune_permission { + my ($rpcenv, $authuser, $storeid, $vmid) = @_; + + if (defined($vmid)) { + $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']); + $rpcenv->check($authuser, "/vms/$vmid", ['VM.Backup']); + } else { + $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']); + } + + return; +} + __PACKAGE__->register_method({ name => 'dryrun', path => '', @@ -20,12 +33,9 @@ __PACKAGE__->register_method({ "Get prune information for backups. NOTE: this is only a preview and might not be " . "what a subsequent prune call does if backups are removed/added in the meantime.", permissions => { - check => [ - 'perm', - '/storage/{storage}', - ['Datastore.Audit', 'Datastore.AllocateSpace'], - any => 1, - ], + description => "You need the 'Datastore.Allocate' privilege on the storage " + . "(or if a VM ID is specified, 'Datastore.AllocateSpace' and 'VM.Backup' for the VM).", + user => 'all', }, protected => 1, proxyto => 'node', @@ -100,6 +110,9 @@ __PACKAGE__->register_method({ code => sub { my ($param) = @_; + my $rpcenv = PVE::RPCEnvironment::get(); + my $authuser = $rpcenv->get_user(); + my $cfg = PVE::Storage::config(); my $vmid = extract_param($param, 'vmid'); @@ -110,6 +123,8 @@ __PACKAGE__->register_method({ $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups) if defined($prune_backups); + assert_prune_permission($rpcenv, $authuser, $storeid, $vmid); + return PVE::Storage::prune_backups($cfg, $storeid, $prune_backups, $vmid, $type, 1); }, }); @@ -177,12 +192,7 @@ __PACKAGE__->register_method({ $prune_backups = PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups) if defined($prune_backups); - if (defined($vmid)) { - $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']); - $rpcenv->check($authuser, "/vms/$vmid", ['VM.Backup']); - } else { - $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']); - } + assert_prune_permission($rpcenv, $authuser, $storeid, $vmid); my $id = (defined($vmid) ? "$vmid@" : '') . $storeid; my $worker = sub { -- 2.47.3