Re: [PATCH pve-firewall v2 1/1] fix #7818: disallow vertical spaces in firewall comments

[Stoiko Ivanov <s.ivanov@proxmox.com>]

Thanks for the quick iteration.

gave it a minimal spin with a pvesh command i used to trigger the issue in
#7818 - catches the new-line as expected.

one nit inline - but with or without that:
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>

On Wed, 15 Jul 2026 12:10:53 +0200
Stefan Hanreich <s.hanreich@proxmox.com> wrote:

> They do not get sanitized and interpreted literally, allowing for
> authenticated users to inject additional lines into the firewall
> configuration files. Additionally restrict the comments to only
> printable characters which additionally prevents control characters in
> comments.
> 
> Suggested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> 
> Notes:
>     Changes from v1 (Thanks @Stoiko):
>     * disallow any vertical space instead of line feeds only
>     * additionally only allow printable characters
>     
>     Did quickly check the other properties as well, but couldn't find
>     anything. Will take a closer look still but wanted to get the patch
>     ready in the meanwhile.
> 
>  src/PVE/Firewall.pm | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 93f8c34..8ec2b80 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1228,6 +1228,22 @@ sub pve_fw_verify_icmp_type_spec {
>      return $icmp_type;
>  }
>  
> +PVE::JSONSchema::register_format('pve-fw-comment-spec', \&pve_fw_verify_comment_spec);
> +
> +sub pve_fw_verify_comment_spec {
> +    my ($comment) = @_;
> +
> +    if ($comment =~ m/\p{VertSpace}/) {
> +        die "comment must not contain any line feeds\n";
nit: would read "line feed" as `\n` only - not general vertical space.
but I think the message is expressive enough - and captures what went
wrong.


> +    }
> +
> +    if ($comment !~ m/^\p{Print}*$/) {
> +        die "comment must only contain printable characters\n";
> +    }
> +
> +    return $comment;
> +}
> +
>  # helper function for API
>  
>  sub copy_opject_with_digest {
> @@ -1623,6 +1639,7 @@ my $rule_properties = {
>          description => "Descriptive comment.",
>          type => 'string',
>          optional => 1,
> +        format => 'pve-fw-comment-spec',
>      },
>      'icmp-type' => {
>          description =>