Re: [PATCH pve-manager] fix #7942: pvesh: treat schema without properties as empty object

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Wed, Aug 19, 2026 at 11:50:23AM +0200, Arthur Bied-Charreton wrote:
> merge_properties() dispatches on the shape of the target schema by
> merging into `properties` if present, appending to an `allOf` list, or
> wrapping a `oneOf` into a new `allOf`. A schema with none of those
> entries died on the assumption that it is not an object.
> 
> A lot of endpoints (e.g., /cluster/firewall/options,
> /cluster/notifications/matcher-field-values) across our api define the
> parameters schema as follows:
> 
>     { additionalProperties => 0 }
> 
> Before 874d160f317a, pvesh treated this as a valid schema allowing no
> parameters. Since that commit, it fails while loading with the
> following error:
> 
>     unknown schema type (not an object?)
> 
> Split the fallback branch into the three cases that can reach it: die
> with the offending type if the schema is explicitly not an object, die
> with the offending keys if it contains unexpected keys, otherwise treat
> `$to` as a valid, empty object and populate `properties` directly.
> 
> Fixes: 874d160f317a ("pvesh: support for allOf/oneOf parameter schemas")
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7942
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  PVE/CLI/pvesh.pm | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/CLI/pvesh.pm b/PVE/CLI/pvesh.pm
> index 0ff481d4..9568e22b 100755
> --- a/PVE/CLI/pvesh.pm
> +++ b/PVE/CLI/pvesh.pm
> @@ -304,6 +304,12 @@ my $our_properties = {
>      },
>  };
>  
> +my sub unknown_schema_keys {
> +    my ($schema) = @_;
> +
> +    return [grep { $_ ne 'additionalProperties' && $_ ne 'type' } keys $schema->%*];
> +}

This does not need to be its own sub.

> +
>  my sub merge_properties {
>      my ($from, $to) = @_;
>  
> @@ -323,7 +329,14 @@ my sub merge_properties {
>          my $old = { $to->%* };
>          $to->%* = (allOf => [$old, $from]);
>      } else {
> -        die "unknown schema type (not an object?)\n";
> +        die "unknown schema type '$to->{type}' (not an object)\n"
> +            if defined($to->{type}) && $to->{type} ne 'object';
> +
> +        my $unknown = unknown_schema_keys($to);
> +        die "unknown schema type, found unexpected keys: " . join(', ', sort($unknown->@*)) . "\n"
> +            if $unknown->@*;

I'm not convinced a list like this is the best way to go here, but it
also won't hurt I suppose.
Either way - please just inline the helper sub, it's a quite specific
part of this check and message.

> +
> +        $to->{properties} = { $from->{properties}->%* };
>      }
>  }
>  
> -- 
> 2.47.3