Re: [PATCH backup 1/2] fix #7054: client: remove trailing newlines from credentials

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On February 20, 2026 12:17 pm, Maximiliano Sandoval wrote:
> For repositories and fingerprints we simply strip trailing whitespaces.
> For passwords, we refer to the password regex at proxmox-schema:
> `^[[:^cntrl:]]*$`, we can only strip trailing control characters without
> potentially breaking existing passwords.
> 
> The encryption password is just a blob of bytes handled locally by the
> client, we cannot remove trailing whitespace here without potential
> breakage. Creation of such passwords (via
> proxmox_sys::tty::read_and_verify_password) only verifies valid utf-8
> and len >= 5.
> 
> In order to prevent an allocation for credentials that do not need to be
> stripped we perform a preemptive check with str::ends_with before
> allocating.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  pbs-client/src/tools/mod.rs | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index 7a496d14c..8dc7c2cd5 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -169,6 +169,17 @@ fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<S
>          Ok(Some(password))
>      } else if let Some(password) = get_credential(credential_name)? {
>          String::from_utf8(password)

nit: while we are here, could we maybe rename this variable? ;)

> +            .map(|s| {
> +                if matches!(credential_name, CRED_PBS_REPOSITORY | CRED_PBS_FINGERPRINT)
> +                    && s.ends_with(char::is_whitespace)

the s.ends_with here, and the trim below are redundant, just trim
below.. this is not a hot path where the slight performance penalty of
trim_end matters, and having two potentially deviating conditions is a
source of bugs.

> +                {
> +                    s.trim_end().to_string()
> +                } else if credential_name == CRED_PBS_PASSWORD && s.ends_with(char::is_control) {
> +                    s.trim_end_matches(char::is_control).to_string()

same here, but this is even more wrong - we should just trim a single
newline, not silently drop control character bytes just because they are
not allowed in passwords! the restriction of the password schema makes
it safe to drop that newline (since it can never be part of the
password).

> +                } else {
> +                    s
> +                }
> +            })
>              .map(Option::Some)
>              .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
>      } else {
> -- 
> 2.47.3
> 
> 
> 
> 
> 
>