Re: [PATCH v2 pve-storage 4/16] api: status: support efi-firmware in upload and download-url

[Fiona Ebner <f.ebner@proxmox.com>]

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 <christian_ludwig@genua.de>
> ---
>  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