From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id C3F4C1FF0B0 for ; Tue, 27 Oct 2026 11:42:46 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C4EBC21656; Tue, 27 Oct 2026 11:42:44 +0100 (CET) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1793097753; x=1793702553; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=yfn+r5bzmawfLKp3fiqbJZ6bayqorbVG9MYV3UyfC8M=; b=rF2oRL02PKId+syV18WrZHDyXNY+rbuq8jqUQdOcJWs7fNMQdWa57Dure+po1XuRHe 8XZAdpAreshO7YqQiiqE2xvhZKWZs7Li98XWG9HKZOHmLrCI5oxhS2UT0Y8J8NZ+MEEu /AL9xJMntWEoj2O1yCNE7VXdYgKr7jO2rEEjoyTQSu2pT6SQVQOrVbC/qz6vgIqtEoVU CxgkcO0VSN5QGj4Z/2Ky42qI8q3STgNLsOcm0oNwmapQ2tpjer2SP8rVdcc90eNkVaA6 7lyu0L9e39P2RfDBFxHkd2mtcUJmAW6u1EAmTUsGhJQxznc7sP7+1GaYS2BvLYeYN+e6 E/UQ== X-Gm-Message-State: AFq9FYLxvC441dnSmGY1O46B7pS99q9EC/SptPr67j1KVxn04P/PjKtK JHCJ5RRwa4aao6fmgt6hzrKe8afdsxTY6QMfX3R7lhev+LE3QGRGtz8o72h3jJuEdJ+B X-Gm-Gg: AYBFou0TNGBhf7RGrxzb5f0t1RNDhqDi8St6BBv/13tWRwg3igum2T0NMn9Pgzmp/qP CZ1LSzT2aLeI6r5BI0YIK2pGTxKt/jlIDVVZSyisbZdl84iwYmTVjYZv+e8E/+vAd+CDK68Tj3A Lsvzvr1Bimc++ThHGZuQPxKYL2nkVG0+CA3D1mOUnmeSDC8zl9gpDyfFtrKXlEfSc7NkKVGDqPs +RoASazdnx7SZWxcV8E7xY9JsCojbLB65d7CF4XakJnunxwZY5O4h1h5Ec2SPQn4esQSpBVKQTp aiTwz/DKjsO8usLgvFwobJQiX4rwpB4syDZfJkaVLrvrqBdwOyznOkTPQvJebBUYrmcUfl2BLY3 jkevhUm1wUWyJOifISjWxMs2nJT6bmt6vTByLFZLAsWmiFEgMFx7EciB9702mn6EKubjkqCaJjd r+DC49f53EdldTxAzFhIADU+YMfZpJrJ9Ck2B6VKYPHEhOqxUxi+imIjf1hA7xkPWTqFKXSsIGs xUWzHKDhDWbM2XW9PvII8CtzLjZMdk8ugeJHQsybfbzcBaNqwi/PY4xJhF9sCbeawFB9Bv2/CVP B8TQNdB1EaVXq+KvGcviWClLmVzZadGQ8OHTaBrLgyntIl19amNYnnY= X-Received: by 2002:a05:600c:5253:b0:4a3:59c7:f226 with SMTP id 5b1f17b1804b1-4a35ac8b40fmr174612595e9.6.1793097408196; Tue, 27 Oct 2026 03:36:48 -0700 (PDT) From: jlcusinato To: pve-devel@lists.proxmox.com Subject: [PATCH qemu-server] fix #7864: api: create vm: honour pool privileges when setting tags Date: Tue, 27 Oct 2026 11:32:34 +0100 Message-ID: <20261027103626.767361-3-jlcusinato@neverhack.com> X-Mailer: git-send-email 2.56.0 In-Reply-To: <20261027103626.767361-1-jlcusinato@neverhack.com> References: <20261027103626.767361-1-jlcusinato@neverhack.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Level: 4 Message-ID-Hash: X7M4GV54MUILZI2NGKKIHH5C4XAH5B7R X-Message-ID-Hash: X7M4GV54MUILZI2NGKKIHH5C4XAH5B7R X-MailFrom: jean.leon.cusinato@gmail.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 CC: felix@ranovus.com, crandler@crandland.de, dreadfulbunny@proton.me X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Jean-Leon CUSINATO Commit a929b3e ("vm_create: check tag permissions if setting tags") started checking tag permissions on the create path, but assert_tag_permissions() resolved VM.Config.Options against a bare /vms/$vmid. At that point the VM is not a pool member yet - add_vm_to_pool() only runs after $realcmd has written the config - so a user or API token whose privileges are granted solely on /pool/ fails with unable to create VM 1021 - 403 Permission check failed (/vms/1021, VM.Config.Options) even though it is allowed to create the VM in that pool, and even though every other create-time option honours the pool via $check_vm_modify_config_perm(). Tags are the only exception, because they are skipped there and checked separately. Pass the pool through, matching the surrounding checks. Setting the same tags in a second config update already worked, so this only removes the need for the caller to hold VM.Config.Options on all of /vms. The old-tags argument passed {} rather than an empty string. PVE::Tools::split_list() stringifies that reference, so with 'user-allow' set to anything but 'free' a pseudo tag like 'HASH(0x...)' enters the per-tag comparison, which no allow list can ever hold since the address is fresh on every call: 403 Permission check failed (/, Sys.Modify for modifying tag 'HASH(0x5c0c4195ab70)') Pass an empty string instead, as the delete branch of the config-update path already does; split_list('') returns an empty list. Fixes: a929b3e ("vm_create: check tag permissions if setting tags") Reported-by: crandler Signed-off-by: Felix Winterhalter Signed-off-by: Jean-Léon CUSINATO --- Compared to the Bugzilla attachment (2057): the old-tags argument is '' rather than {} per crandler's finding in comment #5 of the bug (also CCed) - PVE::Tools::split_list() stringifies a hash reference, so a 'HASH(0x...)' pseudo tag would enter the per-tag comparison whenever 'user-allow' is not 'free'. Verified against current pve-common: split_list('') returns () while split_list({}) yields HASH(0x...). Bug reference #7864 already in the subject; commit message reflowed to the 72-char line-width guideline; rebased onto current master. Resend note (submitted by Jean-Léon CUSINATO ): These patches originate from bug 7864 [0]: code by Felix Winterhalter (the versions attached there, informally reviewed by a Proxmox maintainer in comment #4, who asked for a posting on this list) plus crandler's follow-up finding (comment #5) and dreadfulbunny's data point on the container create path (comment #7). Neither original author has been active on the bug since September 2026, and the list never received a posting despite that request, so I am submitting them here with their attribution intact in the commit messages (Signed-off-by / Reported-by). All contributors are CCed. My individual CLA has been submitted to and recorded by Proxmox (office@proxmox.com, October 2026), who asked for the patches to be posted here. Not runtime-tested (no test node available); verification is code-level only. Feedback, especially testing, welcome. Series merge order: 1) pve-guest-common, then 2) qemu-server and 3) pve-container (the latter two only rely on the new optional $pool parameter introduced in 1). [0] https://bugzilla.proxmox.com/show_bug.cgi?id=7864 src/PVE/API2/Qemu.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm index f4dbd2a..5789f99 100644 --- a/src/PVE/API2/Qemu.pm +++ b/src/PVE/API2/Qemu.pm @@ -1568,7 +1568,7 @@ __PACKAGE__->register_method({ if (defined $conf->{tags}) { assert_tag_permissions( - $vmid, {}, $conf->{tags}, $rpcenv, $authuser, + $vmid, '', $conf->{tags}, $rpcenv, $authuser, $pool, ); $conf->{tags} = PVE::GuestHelpers::get_unique_tags($conf->{tags}); } -- 2.56.0