Re: [PATCH proxmox-backup v3 03/15] pbs-config: use proxmox-product-config::replace_secret_config()

[Robert Obkircher <r.obkircher@proxmox.com>]

On 01.07.26 16:04, Christian Ebner wrote:
> Instead of using the pbs-config local implementation, use the product
> general implementation, dropping the local one instead.
>
> Since proxmox-product-config::replace_secret_config() requires the
> api- and priv-user to be initialized, any calling codepath must
> guarantee to have run proxmox-product-config::init() once, so do
> that for proxmox-tape as well, while proxy, api and manager are
> already initializing it.
>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  pbs-config/Cargo.toml           |  1 +
>  pbs-config/src/lib.rs           | 17 -----------------
>  pbs-config/src/notifications.rs |  5 ++++-
>  src/bin/proxmox-tape.rs         |  8 ++++++++
>  src/tape/encryption_keys.rs     |  3 ++-
>  5 files changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/pbs-config/Cargo.toml b/pbs-config/Cargo.toml
> index 04687cb59..d07b4de89 100644
> --- a/pbs-config/Cargo.toml
> +++ b/pbs-config/Cargo.toml
> @@ -23,6 +23,7 @@ proxmox-http.workspace = true
>  proxmox-lang.workspace = true
>  proxmox-notify.workspace = true
>  proxmox-router = { workspace = true, default-features = false }
> +proxmox-product-config.workspace = true
>  proxmox-s3-client.workspace = true
>  proxmox-schema.workspace = true
>  proxmox-section-config.workspace = true
> diff --git a/pbs-config/src/lib.rs b/pbs-config/src/lib.rs
> index a194d70ac..1d32d23e2 100644
> --- a/pbs-config/src/lib.rs
> +++ b/pbs-config/src/lib.rs
> @@ -139,23 +139,6 @@ pub fn replace_backup_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) ->
>      Ok(())
>  }
>  
> -/// Atomically write data to file owned by "root:root" with permission "0600"
> -///
> -/// Only the superuser can read and write those files.
> -pub fn replace_secret_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
> -    let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
> -    // set the correct owner/group/permissions while saving file
> -    // owner(rw) = root, group(r)= root
> -    let options = proxmox_sys::fs::CreateOptions::new()
> -        .perm(mode)
> -        .owner(nix::unistd::ROOT)
> -        .group(nix::unistd::Gid::from_raw(0));
> -
> -    proxmox_sys::fs::replace_file(path, data, options, true)?;
> -
> -    Ok(())
> -}
> -
>  /// Detect modified configuration files
>  ///
>  /// This function fails with a reasonable error message if checksums do not match.
> diff --git a/pbs-config/src/notifications.rs b/pbs-config/src/notifications.rs
> index 3ee019f23..cbdbcee7a 100644
> --- a/pbs-config/src/notifications.rs
> +++ b/pbs-config/src/notifications.rs
> @@ -35,7 +35,10 @@ pub fn config() -> Result<Config, Error> {
>  pub fn save_config(config: Config) -> Result<(), Error> {
>      let (cfg, priv_cfg) = config.write()?;
>      crate::replace_backup_config(NOTIFICATION_CONFIG_PATH, cfg.as_bytes())?;
> -    crate::replace_secret_config(NOTIFICATION_PRIV_CONFIG_PATH, priv_cfg.as_bytes())?;
> +    proxmox_product_config::replace_secret_config(
> +        NOTIFICATION_PRIV_CONFIG_PATH,
> +        priv_cfg.as_bytes(),
> +    )?;
It is probably fine, but priv_user is no longer guaranteed to be root
(in cfg(test)) 
> [..]