Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions crates/admin-cli/src/bmc_machine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ mod enable_infinite_boot;
mod is_infinite_boot_enabled;
mod lockdown;
mod lockdown_status;
mod probe_vendor;
mod set_root_password;

#[cfg(test)]
mod tests;
Expand All @@ -49,4 +51,10 @@ pub enum Cmd {
Lockdown(lockdown::Args),
#[clap(about = "Check lockdown status")]
LockdownStatus(lockdown_status::Args),
#[clap(
about = "Set a BMC's root password out-of-band (for fleet rotation use `credential rotate`)"
)]
SetRootPassword(set_root_password::Args),
#[clap(about = "Resolve a BMC's Redfish vendor")]
ProbeVendor(probe_vendor::Args),
}
61 changes: 61 additions & 0 deletions crates/admin-cli/src/bmc_machine/probe_vendor/args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

use clap::Parser;
use mac_address::MacAddress;
use rpc::forge as forgerpc;

#[derive(Parser, Debug, Clone)]
#[command(after_long_help = "\
EXAMPLES:
Probe a BMC's Redfish vendor, targeting the BMC by machine id:
$ nico-admin-cli bmc-machine probe-vendor --machine 12345678-1234-5678-90ab-cdef01234567
Target the BMC by IP address:
$ nico-admin-cli bmc-machine probe-vendor --ip-address 192.0.2.20
Target the BMC by MAC address:
$ nico-admin-cli bmc-machine probe-vendor --mac-address 00:11:22:33:44:55
Comment thread
coderabbitai[bot] marked this conversation as resolved.
")]
pub struct Args {
#[clap(long, short, help = "IP of the BMC whose vendor to probe")]
pub ip_address: Option<String>,
#[clap(long, help = "MAC of the BMC whose vendor to probe")]
pub mac_address: Option<MacAddress>,
#[clap(long, short, help = "ID of the machine whose BMC vendor to probe")]
pub machine: Option<String>,
}

impl From<Args> for forgerpc::ProbeBmcVendorRequest {
fn from(args: Args) -> Self {
let bmc_endpoint_request = if args.ip_address.is_some() || args.mac_address.is_some() {
Some(forgerpc::BmcEndpointRequest {
ip_address: args.ip_address.unwrap_or_default(),
mac_address: args.mac_address.map(|mac| mac.to_string()),
})
} else {
None
};

Self {
bmc_endpoint_request,
machine_id: args.machine,
}
}
}
26 changes: 26 additions & 0 deletions crates/admin-cli/src/bmc_machine/probe_vendor/cmd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

use super::args::Args;
use crate::errors::CarbideCliResult;
use crate::rpc::ApiClient;

pub async fn probe_vendor(args: Args, api_client: &ApiClient) -> CarbideCliResult<()> {
let response = api_client.0.probe_bmc_vendor(args).await?;
println!("{}", response.vendor);
Ok(())
}
31 changes: 31 additions & 0 deletions crates/admin-cli/src/bmc_machine/probe_vendor/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

pub mod args;
pub mod cmd;

pub use args::Args;

use crate::cfg::run::Run;
use crate::cfg::runtime::RuntimeContext;
use crate::errors::CarbideCliResult;

impl Run for Args {
async fn run(self, ctx: &mut RuntimeContext) -> CarbideCliResult<()> {
cmd::probe_vendor(self, &ctx.api_client).await
}
}
68 changes: 68 additions & 0 deletions crates/admin-cli/src/bmc_machine/set_root_password/args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

use clap::Parser;
use mac_address::MacAddress;
use rpc::forge as forgerpc;

#[derive(Parser, Debug, Clone)]
#[command(after_long_help = "\
EXAMPLES:

Rotate the BMC root password, targeting the BMC by machine id:
$ nico-admin-cli bmc-machine set-root-password \
--machine 12345678-1234-5678-90ab-cdef01234567 --new-password mynewpassword

Target the BMC by IP address:
$ nico-admin-cli bmc-machine set-root-password \
--ip-address 192.0.2.20 --new-password mynewpassword

Target the BMC by MAC address:
$ nico-admin-cli bmc-machine set-root-password \
--mac-address 00:11:22:33:44:55 --new-password mynewpassword
Comment thread
coderabbitai[bot] marked this conversation as resolved.

")]
pub struct Args {
#[clap(long, short, help = "IP of the BMC whose root password to set")]
pub ip_address: Option<String>,
#[clap(long, help = "MAC of the BMC whose root password to set")]
pub mac_address: Option<MacAddress>,
#[clap(long, short, help = "ID of the machine whose BMC root password to set")]
pub machine: Option<String>,

#[clap(long, help = "New BMC root password to set")]
pub new_password: String,
}

impl From<Args> for forgerpc::SetBmcRootPasswordRequest {
fn from(args: Args) -> Self {
let bmc_endpoint_request = if args.ip_address.is_some() || args.mac_address.is_some() {
Some(forgerpc::BmcEndpointRequest {
ip_address: args.ip_address.unwrap_or_default(),
mac_address: args.mac_address.map(|mac| mac.to_string()),
})
} else {
None
};

Self {
bmc_endpoint_request,
machine_id: args.machine,
new_password: args.new_password,
}
}
}
26 changes: 26 additions & 0 deletions crates/admin-cli/src/bmc_machine/set_root_password/cmd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

use super::args::Args;
use crate::errors::CarbideCliResult;
use crate::rpc::ApiClient;

pub async fn set_root_password(args: Args, api_client: &ApiClient) -> CarbideCliResult<()> {
api_client.0.set_bmc_root_password(args).await?;
println!("BMC root password updated");
Ok(())
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
31 changes: 31 additions & 0 deletions crates/admin-cli/src/bmc_machine/set_root_password/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

pub mod args;
pub mod cmd;

pub use args::Args;

use crate::cfg::run::Run;
use crate::cfg::runtime::RuntimeContext;
use crate::errors::CarbideCliResult;

impl Run for Args {
async fn run(self, ctx: &mut RuntimeContext) -> CarbideCliResult<()> {
cmd::set_root_password(self, &ctx.api_client).await
}
}
14 changes: 14 additions & 0 deletions crates/api-core/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2862,6 +2862,20 @@ impl Forge for Api {
crate::handlers::bmc_endpoint_explorer::create_bmc_user(self, request).await
}

async fn set_bmc_root_password(
&self,
request: Request<rpc::SetBmcRootPasswordRequest>,
) -> Result<Response<rpc::SetBmcRootPasswordResponse>, Status> {
crate::handlers::bmc_endpoint_explorer::set_bmc_root_password(self, request).await
}

async fn probe_bmc_vendor(
&self,
request: Request<rpc::ProbeBmcVendorRequest>,
) -> Result<Response<rpc::ProbeBmcVendorResponse>, Status> {
crate::handlers::bmc_endpoint_explorer::probe_bmc_vendor(self, request).await
}

async fn delete_bmc_user(
&self,
request: Request<rpc::DeleteBmcUserRequest>,
Expand Down
69 changes: 69 additions & 0 deletions crates/api-core/src/handlers/bmc_endpoint_explorer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,75 @@ pub(crate) async fn delete_bmc_user(
Ok(Response::new(rpc::DeleteBmcUserResponse {}))
}

pub(crate) async fn set_bmc_root_password(
api: &Api,
request: Request<rpc::SetBmcRootPasswordRequest>,
) -> Result<Response<rpc::SetBmcRootPasswordResponse>, Status> {
log_request_data(&request);
let req = request.into_inner();
Comment on lines +988 to +993

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -A 15 'fn log_request_data' --type=rust
rg -n 'new_password' crates/rpc/proto/forge.proto

Repository: NVIDIA/infra-controller

Length of output: 161


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- bmc_endpoint_explorer outline ---'
ast-grep outline crates/api-core/src/handlers/bmc_endpoint_explorer.rs --view expanded || true

echo '--- handler slice ---'
sed -n '980,1065p' crates/api-core/src/handlers/bmc_endpoint_explorer.rs | cat -n

echo '--- log_request_data search ---'
rg -n -A 25 -B 5 'fn log_request_data|log_request_data\(' crates/api-core crates -g '!target' || true

echo '--- proto field search ---'
rg -n -A 5 -B 5 'new_password|SetBmcRootPasswordRequest|SetBmcRootPasswordResponse' crates -g '!target' || true

Repository: NVIDIA/infra-controller

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- handler excerpt ---'
sed -n '988,1065p' crates/api-core/src/handlers/bmc_endpoint_explorer.rs | cat -n

echo '--- log_request_data definition(s) ---'
rg -n -A 40 -B 10 'fn log_request_data' crates/api-core crates -g '!target' | sed -n '1,220p'

echo '--- request schema for password field ---'
rg -n -A 30 -B 10 'message SetBmcRootPasswordRequest|new_password|current_password|password' crates/rpc crates -g '!target' | sed -n '1,220p'

echo '--- EndpointExplorationError mapping ---'
rg -n -A 40 -B 10 'enum EndpointExplorationError|match .*EndpointExplorationError|CarbideError::internal\(e\.to_string\(\)\)' crates/api-core/src/handlers/bmc_endpoint_explorer.rs crates -g '!target' | sed -n '1,260p'

Repository: NVIDIA/infra-controller

Length of output: 50379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -A 40 -B 10 'enum EndpointExplorationError|type EndpointExplorationError|struct .*EndpointExploration|impl .*EndpointExplorationError|set_bmc_root_password\(' crates/api-core crates -g '!target' | sed -n '1,240p'

Repository: NVIDIA/infra-controller

Length of output: 22184


Redact new_password before request logging. log_request_data(&request) records request.get_ref() via Debug here, so the plaintext BMC root password can end up in the span. Switch to log_request_data_redacted(...) or log a redacted copy after into_inner() so credentials never reach logs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/api-core/src/handlers/bmc_endpoint_explorer.rs` around lines 988 -
993, The request logging in set_bmc_root_password is exposing the plaintext
new_password because log_request_data(&request) logs the full Debug payload
before redaction. Update this path to use log_request_data_redacted(...) or
first call into_inner() and log a redacted copy of
rpc::SetBmcRootPasswordRequest so the sensitive field is never written to logs.

Source: Path instructions


let machine_id = req
.machine_id
.as_ref()
.map(|id| try_parse_machine_id(id))
.transpose()?;

let mut txn = api.txn_begin().await?;
let (bmc_endpoint_request, _) =
validate_and_complete_bmc_endpoint_request(&mut txn, req.bmc_endpoint_request, machine_id)
.await?;
txn.commit().await?;

let (bmc_addr, bmc_mac_address) = resolve_bmc_interface(api, &bmc_endpoint_request).await?;
let machine_interface = MachineInterfaceSnapshot::mock_with_mac(bmc_mac_address);

tracing::info!(%bmc_addr, "Setting BMC root password");

api.endpoint_explorer
.set_bmc_root_password(bmc_addr, &machine_interface, &req.new_password)
.await
.map_err(|e| CarbideError::internal(e.to_string()))?;
Comment on lines +1012 to +1015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Blanket CarbideError::internal mapping loses the invalid-argument vs system-error distinction.

Both new handlers collapse every EndpointExplorationError (e.g. bad current password/Unauthorized, missing vault entry/MissingCredentials, genuine network/BMC failure) into CarbideError::internal. Callers (admin-cli, other services) can no longer distinguish "you supplied the wrong current password" (a 4xx-like, user-actionable condition) from "the BMC/network is down" (a genuine 5xx). This is the exact "avoid — constructing Status directly, bypassing NicoError error mapping" anti-pattern flagged in the referenced STYLE_GUIDE, applied in spirit to error variant selection.

Consider mapping the underlying EndpointExplorationError to the appropriate CarbideError variant (similar to how map_redfish_client_creation_error/map_redfish_error differentiate causes elsewhere in redfish.rs) instead of a single catch-all .internal().

As per path instructions, "Prefer NicoError-based error construction inside handlers... choose the NicoError variant based on whether failures are user-input/invalid-argument (4xx-like) vs internal/system issues."

Also applies to: 1044-1048

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/api-core/src/handlers/bmc_endpoint_explorer.rs` around lines 1012 -
1015, The handler logic in the endpoint explorer is collapsing all
EndpointExplorationError cases into CarbideError::internal, which loses the
user-input vs system-failure distinction. Update the error mapping in the root
password flow and the other affected handler to inspect the underlying
EndpointExplorationError and return the appropriate CarbideError variant for bad
password/missing credential cases versus genuine BMC/network failures. Follow
the same pattern used by map_redfish_client_creation_error and map_redfish_error
so callers can reliably distinguish actionable 4xx-like errors from internal
5xx-like errors.

Source: Path instructions


tracing::info!(%bmc_addr, "Successfully set BMC root password");

Ok(Response::new(rpc::SetBmcRootPasswordResponse {}))
}

pub(crate) async fn probe_bmc_vendor(
api: &Api,
request: Request<rpc::ProbeBmcVendorRequest>,
) -> Result<Response<rpc::ProbeBmcVendorResponse>, Status> {
log_request_data(&request);
let req = request.into_inner();

let machine_id = req
.machine_id
.as_ref()
.map(|id| try_parse_machine_id(id))
.transpose()?;

let mut txn = api.txn_begin().await?;
let (bmc_endpoint_request, _) =
validate_and_complete_bmc_endpoint_request(&mut txn, req.bmc_endpoint_request, machine_id)
.await?;
txn.commit().await?;

let (bmc_addr, bmc_mac_address) = resolve_bmc_interface(api, &bmc_endpoint_request).await?;
let machine_interface = MachineInterfaceSnapshot::mock_with_mac(bmc_mac_address);

let vendor = api
.endpoint_explorer
.probe_bmc_vendor(bmc_addr, &machine_interface)
.await
.map_err(|e| CarbideError::internal(e.to_string()))?;

tracing::info!(%bmc_addr, %vendor, "Probed BMC vendor");

Ok(Response::new(rpc::ProbeBmcVendorResponse {
vendor: vendor.to_string(),
}))
}

async fn do_create_bmc_user(
api: &Api,
request: &rpc::BmcEndpointRequest,
Expand Down
Loading
Loading