Re: [PATCH proxmox-backup] fix #7911: api: report: mark endpoint as protected

[Erik Fastermann <e.fastermann@proxmox.com>]

On 8/28/26 9:41 AM, Christian Ebner wrote:
> On 8/11/26 11:36 AM, Erik Fastermann wrote:
>> The report was generated by proxmox-backup-proxy, which runs as the
>> unprivileged 'backup' user, so commands needing root only partly
>> worked. ethtool printed "netlink error: Operation not permitted" to
>> stderr, which ended up in the report. dmidecode and proxmox-boot-tool
>> failed completely.
>>
>> Forward the request to the privileged API daemon instead, like PVE,
>> PMG and PDM already do. Users with Sys.Audit thus see slightly more
>> host details, which the other products already accept.
> 
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7911
> 
>> Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
> 
> Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
> Tested-by: Christian Ebner <c.ebner@proxmox.com>
> 

Thanks for the review and test!

> One question which came to mind during review is if we should maybe also 
> add stricter type checks on the allowed parameters returned by 
> dynamic_commands()?
> 
> We do allow arbitrary strings to be returned there. While the current 
> use-case is protected by proxmox-network-api config parsing, I think it 
> would make sense to restrict this to avoid potential future miss-use and 
> potential code execution by injection (after all one runs as privileged 
> user now).
> 
> Maybe we could add an enum with allowed command -> args pairs? The args 
> assured to be type checked?

I'd rather not. Could only cover the narrowest of the three collection 
paths and e.g. function_calls() already runs arbitrary Rust, including 
spawning 'top'. Constraining the dynamic arguments while that stays buys 
little.

I'm also not sure how the type checked pairs would look without becoming 
unwieldy and we will probably rarely touch the code in practice.

Note also that after the shared crate series [0], dynamic_commands() 
lives in proxmox-system-report as common_dynamic_commands() and 
DynamicArgsCommandSpec is private there. generate_report() only accepts 
StaticArgsCommandSpec from the products, so the dynamic surface is a 
single crate-internal function.

[0] 
https://lore.proxmox.com/pbs-devel/20260811114332.283776-1-e.fastermann@proxmox.com/

>> --- a/src/api2/node/report.rs
>> +++ b/src/api2/node/report.rs
>> @@ -9,6 +9,7 @@ use pbs_api_types::{NODE_SCHEMA, PRIV_SYS_AUDIT};
>>   use crate::server::generate_report;
>>   #[api(
>> +    protected: true,
>>       input: {
>>           properties: {
>>               node: {
>