Re: [pve-devel] [PATCH storage v2] fix #5181: pbs: store and read passwords as unicode

From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Cc: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH storage v2] fix #5181: pbs: store and read passwords as unicode
Date: Wed, 23 Jul 2025 15:00:32 +0200	[thread overview]
Message-ID: <DBJGCAE47P9V.A3OVL24G5EDG@proxmox.com> (raw)
In-Reply-To: <20250613131125.354560-1-m.sandoval@proxmox.com>

-->8 snip 8<--
>
> -    PVE::Tools::file_set_contents($pwfile, "$password\n");
> +    PVE::Tools::file_set_contents($pwfile, "$password\n", undef, 1);

i know this is pre-existing, but i'd feel more comfortable forcing the
permissions here rather than depending on the default behaviour. this is
a password file after all, being explicit doesn't hurt in my opinion.

>  }
>
>  sub pbs_delete_password {
> @@ -109,7 +110,9 @@ sub pbs_get_password {
>
>      my $pwfile = pbs_password_file_name($scfg, $storeid);
>
> -    return PVE::Tools::file_read_firstline($pwfile);
> +    my $contents = PVE::Tools::file_read_firstline($pwfile);
> +
> +    return eval { decode('UTF-8', $contents, 1) } // $contents;
>  }
>
>  sub pbs_encryption_key_file_name {

other than the one comment above, consider this:

Reviewed-by: Shannon Sterz <s.sterz@proxmox.com>
Tested-by: Shannon Sterz <s.sterz@proxmox.com>

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel