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

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

[Arthur Bied-Charreton]

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->%*];
+}
+
 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->@*;
+
+        $to->{properties} = { $from->{properties}->%* };
     }
 }
 
-- 
2.47.3

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

[Wolfgang Bumiller]

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

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

[Arthur Bied-Charreton]

On Tue, Sep 01, 2026 at 05:32:04PM +0200, Wolfgang Bumiller wrote:
> 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.
> 
fair enough, will inline it in v2
> > +
> >  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.
i'm not very set on this, just thought it would make sense to include
the offending keys in the error message. do you have another option in
mind? 
> Either way - please just inline the helper sub, it's a quite specific
> part of this check and message.
ack, see above
> 
> > +
> > +        $to->{properties} = { $from->{properties}->%* };
> >      }
> >  }
> >  
> > -- 
> > 2.47.3

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

[Arthur Bied-Charreton]

Superseded by:
https://lore.proxmox.com/pve-devel/20260907094454.242395-1-a.bied-charreton@proxmox.com/T/#u