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 008791FF09C for ; Mon, 05 Oct 2026 16:37:28 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A73962167B; Mon, 05 Oct 2026 16:37:24 +0200 (CEST) Message-ID: Date: Mon, 5 Oct 2026 16:37:04 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 pve-storage 4/16] api: status: support efi-firmware in upload and download-url To: Christian Ludwig , pve-devel@lists.proxmox.com References: <62b59850da5ca40be46662b94e10d5499e615c04.1790337423.git@genua.de> Content-Language: en-US From: Fiona Ebner In-Reply-To: <62b59850da5ca40be46662b94e10d5499e615c04.1790337423.git@genua.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791211024912 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.472 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: 6DW6O2V766SH23EJKOTFSSZMU3PMYXIR X-Message-ID-Hash: 6DW6O2V766SH23EJKOTFSSZMU3PMYXIR 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: Am 28.09.26 um 7:49 AM schrieb Christian Ludwig: > Allow the content type in both endpoints and reject file names outside > the safe character class. > > Signed-off-by: Christian Ludwig > --- > src/PVE/API2/Storage/Status.pm | 14 ++++++++++++-- > src/PVE/Storage.pm | 18 ++++++++++++++++-- > 2 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm > index 741d514..a6fc317 100644 > --- a/src/PVE/API2/Storage/Status.pm > +++ b/src/PVE/API2/Storage/Status.pm > @@ -533,7 +533,7 @@ __PACKAGE__->register_method({ > description => "Content type.", > type => 'string', > format => 'pve-storage-content', > - enum => ['iso', 'vztmpl', 'import'], > + enum => ['iso', 'vztmpl', 'import', 'efi-firmware'], > }, > filename => { > description => > @@ -618,6 +618,11 @@ __PACKAGE__->register_method({ > } > > $path = PVE::Storage::get_import_dir($cfg, $storage); > + } elsif ($content eq 'efi-firmware') { > + if ($filename !~ m!${PVE::Storage::SAFE_CHAR_CLASS_RE}+$!) { Note: if we go for enforcing extensions as suggested in patch 1/16, this needs to be adapted as well. > + raise_param_exc({ filename => "invalid file name" }); > + } > + $path = PVE::Storage::get_efi_firmware_dir($cfg, $storage); > } else { > raise_param_exc({ content => "upload content type '$content' not allowed" }); > } I'd like to have the image be validated below, where validation for other types already happens, with something like: my ($undef, $format) = file_size_info($tmpfilename, 10, 'auto-detect', 1); die "..." if $format ne 'raw'; > @@ -770,7 +775,7 @@ __PACKAGE__->register_method({ > description => "Content type.", # TODO: could be optional & detected in most cases > type => 'string', > format => 'pve-storage-content', > - enum => ['iso', 'vztmpl', 'import'], > + enum => ['iso', 'vztmpl', 'import', 'efi-firmware'], > }, > filename => { > description => > @@ -859,6 +864,11 @@ __PACKAGE__->register_method({ > } > > $path = PVE::Storage::get_import_dir($cfg, $storage); > + } elsif ($content eq 'efi-firmware') { > + if ($filename !~ m!${PVE::Storage::SAFE_CHAR_CLASS_RE}+$!) { > + raise_param_exc({ filename => "invalid file name" }); > + } > + $path = PVE::Storage::get_efi_firmware_dir($cfg, $storage); Similar comments as above apply here too. > } else { > raise_param_exc({ content => "upload content-type '$content' is not allowed" }); > } > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm > index 64ea9da..112a828 100755 > --- a/src/PVE/Storage.pm > +++ b/src/PVE/Storage.pm > @@ -555,6 +555,15 @@ sub get_iso_dir { > return $plugin->get_subdir($scfg, 'iso'); > } > > +sub get_efi_firmware_dir { > + my ($cfg, $storeid) = @_; > + > + my $scfg = storage_config($cfg, $storeid); > + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); > + > + return $plugin->get_subdir($scfg, 'efi-firmware'); > +} > + > sub get_import_dir { > my ($cfg, $storeid) = @_; > > @@ -629,7 +638,12 @@ sub check_volume_access { > > return if $rpcenv->check($user, "/storage/$sid", ['Datastore.Allocate'], 1); > > - if ($vtype eq 'iso' || $vtype eq 'vztmpl' || $vtype eq 'import') { > + if ( > + $vtype eq 'iso' > + || $vtype eq 'vztmpl' > + || $vtype eq 'import' > + || $vtype eq 'efi-firmware' > + ) { This hunk belongs to patch 1/16. > # require at least read access to storage, (custom) templates/ISOs could be sensitive > $rpcenv->check_any( > $user, > @@ -1297,7 +1311,7 @@ sub template_list { > sub volume_list { > my ($cfg, $storeid, $vmid, $content) = @_; > > - my @ctypes = qw(rootdir images vztmpl iso backup snippets import); > + my @ctypes = qw(rootdir images vztmpl iso backup snippets import efi-firmware); This hunk belongs to patch 1/16. > > my $cts = $content ? [$content] : [@ctypes]; > Best Regards, Fiona