Skip to content

Commit bd3c936

Browse files
committed
ommit --server-id in firewall and cred commands when lock file for cloud server exists
1 parent 9d995d2 commit bd3c936

4 files changed

Lines changed: 390 additions & 64 deletions

File tree

src/console/commands/cli/agent.rs

Lines changed: 204 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
1212
use crate::cli::config_bundle::{build_config_bundle, ConfigBundleArtifacts};
1313
use crate::cli::config_parser::StackerConfig;
14+
use crate::cli::debug::cli_debug_enabled;
1415
use crate::cli::error::CliError;
1516
use crate::cli::fmt;
1617
use crate::cli::generator::compose::ComposeDefinition;
@@ -186,9 +187,35 @@ fn json_error_message(value: &serde_json::Value) -> Option<String> {
186187
}
187188
}
188189

190+
fn sanitize_npm_credentials_message(raw_message: String, code: Option<&str>) -> String {
191+
// Fall back to substring match when the error arrives as a pre-formatted string
192+
// with no structured "code" field (the server embeds the code inline).
193+
if code == Some("npm_credentials_invalid")
194+
|| raw_message.contains("npm_credentials_invalid")
195+
{
196+
let user_msg = "NPM credentials are invalid or missing. \
197+
Update them with:\n \
198+
stacker secrets set npm_credentials --scope server \
199+
--body-file ./npm_credentials.json"
200+
.to_string();
201+
if cli_debug_enabled() {
202+
format!("{}\n [debug] {}", user_msg, raw_message)
203+
} else {
204+
user_msg
205+
}
206+
} else {
207+
raw_message
208+
}
209+
}
210+
189211
fn agent_command_error_message(info: &AgentCommandInfo) -> Option<String> {
190212
if let Some(error) = info.error.as_ref() {
191-
return json_error_message(error).or_else(|| Some(fmt::pretty_json(error)));
213+
let raw = json_error_message(error).unwrap_or_else(|| fmt::pretty_json(error));
214+
let code = error
215+
.get("code")
216+
.and_then(|v| v.as_str())
217+
.or_else(|| error.get("error_code").and_then(|v| v.as_str()));
218+
return Some(sanitize_npm_credentials_message(raw, code));
192219
}
193220

194221
let result = info.result.as_ref()?;
@@ -201,16 +228,28 @@ fn agent_command_error_message(info: &AgentCommandInfo) -> Option<String> {
201228
return None;
202229
}
203230

204-
let mut message = json_error_message(result)
231+
let raw_message = json_error_message(result)
205232
.unwrap_or_else(|| "Agent command reported an application error".to_string());
206-
if let Some(code) = result.get("error_code").and_then(|value| value.as_str()) {
207-
message = format!("{} ({})", message, code);
208-
if code == "npm_create_failed" {
209-
message = format!(
210-
"{}\n\n{}",
211-
message,
212-
npm_create_failed_guidance(Some(result))
213-
);
233+
234+
// "code" is already embedded into raw_message by format_error_message.
235+
// "error_code" is a separate field not yet appended — handled below.
236+
let inline_code = result.get("code").and_then(|v| v.as_str());
237+
let extra_code = result.get("error_code").and_then(|v| v.as_str());
238+
239+
let mut message = sanitize_npm_credentials_message(raw_message, inline_code.or(extra_code));
240+
241+
// Append extra_code (the "error_code" field) if present — it is NOT yet in the message.
242+
if let Some(code) = extra_code {
243+
// Skip appending if sanitize_npm_credentials_message already replaced the whole message.
244+
if inline_code != Some("npm_credentials_invalid") {
245+
message = format!("{} ({})", message, code);
246+
if code == "npm_create_failed" {
247+
message = format!(
248+
"{}\n\n{}",
249+
message,
250+
npm_create_failed_guidance(Some(result))
251+
);
252+
}
214253
}
215254
}
216255
Some(message)
@@ -3227,6 +3266,161 @@ monitoring:
32273266
);
32283267
}
32293268

3269+
// Shared lock so env-var tests don't race each other.
3270+
fn npm_creds_env_lock() -> std::sync::MutexGuard<'static, ()> {
3271+
use std::sync::{Mutex, OnceLock};
3272+
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
3273+
LOCK.get_or_init(|| Mutex::new(())).lock().unwrap()
3274+
}
3275+
3276+
fn npm_creds_invalid_info_via_result(vault_path: &str) -> AgentCommandInfo {
3277+
AgentCommandInfo {
3278+
command_id: "cmd_npm_creds".to_string(),
3279+
deployment_hash: "dep".to_string(),
3280+
command_type: "configure_proxy".to_string(),
3281+
status: "completed".to_string(),
3282+
priority: "normal".to_string(),
3283+
parameters: None,
3284+
result: Some(serde_json::json!({
3285+
"status": "error",
3286+
"code": "npm_credentials_invalid",
3287+
"message": format!("NPM credentials in Vault are invalid at {}", vault_path),
3288+
"details": vault_path,
3289+
})),
3290+
error: None,
3291+
created_at: String::new(),
3292+
updated_at: String::new(),
3293+
}
3294+
}
3295+
3296+
fn npm_creds_invalid_info_via_error(vault_path: &str) -> AgentCommandInfo {
3297+
AgentCommandInfo {
3298+
command_id: "cmd_npm_creds".to_string(),
3299+
deployment_hash: "dep".to_string(),
3300+
command_type: "configure_proxy".to_string(),
3301+
status: "failed".to_string(),
3302+
priority: "normal".to_string(),
3303+
parameters: None,
3304+
result: None,
3305+
error: Some(serde_json::json!({
3306+
"code": "npm_credentials_invalid",
3307+
"message": format!("NPM credentials in Vault are invalid at {}", vault_path),
3308+
"details": vault_path,
3309+
})),
3310+
created_at: String::new(),
3311+
updated_at: String::new(),
3312+
}
3313+
}
3314+
3315+
#[test]
3316+
fn agent_command_error_message_sanitizes_vault_path_via_result_field() {
3317+
let _guard = npm_creds_env_lock();
3318+
std::env::remove_var("STACKER_DEBUG");
3319+
std::env::remove_var("DEBUG");
3320+
3321+
let vault_path = "secret/base/status_panel/hosts/86/npm_credentials";
3322+
let message = agent_command_error_message(&npm_creds_invalid_info_via_result(vault_path))
3323+
.expect("error message");
3324+
3325+
assert!(
3326+
!message.contains(vault_path),
3327+
"Vault path must not appear in user-facing output: {message}"
3328+
);
3329+
assert!(
3330+
message.contains("stacker secrets set npm_credentials"),
3331+
"Message should include the remediation command: {message}"
3332+
);
3333+
}
3334+
3335+
#[test]
3336+
fn agent_command_error_message_sanitizes_vault_path_via_error_field() {
3337+
let _guard = npm_creds_env_lock();
3338+
std::env::remove_var("STACKER_DEBUG");
3339+
std::env::remove_var("DEBUG");
3340+
3341+
let vault_path = "secret/base/status_panel/hosts/86/npm_credentials";
3342+
let message = agent_command_error_message(&npm_creds_invalid_info_via_error(vault_path))
3343+
.expect("error message");
3344+
3345+
assert!(
3346+
!message.contains(vault_path),
3347+
"Vault path must not appear in user-facing output (error field path): {message}"
3348+
);
3349+
assert!(
3350+
message.contains("stacker secrets set npm_credentials"),
3351+
"Message should include the remediation command: {message}"
3352+
);
3353+
}
3354+
3355+
#[test]
3356+
fn agent_command_error_message_sanitizes_vault_path_when_error_is_preformatted_string() {
3357+
let _guard = npm_creds_env_lock();
3358+
std::env::remove_var("STACKER_DEBUG");
3359+
std::env::remove_var("DEBUG");
3360+
3361+
let vault_path = "secret/base/status_panel/hosts/86/npm_credentials";
3362+
// Simulate the server sending a pre-formatted string (no structured "code" field)
3363+
let info = AgentCommandInfo {
3364+
command_id: "cmd_npm_creds".to_string(),
3365+
deployment_hash: "dep".to_string(),
3366+
command_type: "configure_proxy".to_string(),
3367+
status: "failed".to_string(),
3368+
priority: "normal".to_string(),
3369+
parameters: None,
3370+
result: None,
3371+
error: Some(serde_json::Value::String(format!(
3372+
"NPM credentials in Vault are invalid at {vault_path} (npm_credentials_invalid): {vault_path}"
3373+
))),
3374+
created_at: String::new(),
3375+
updated_at: String::new(),
3376+
};
3377+
3378+
let message = agent_command_error_message(&info).expect("error message");
3379+
3380+
assert!(
3381+
!message.contains(vault_path),
3382+
"Vault path must not appear when error is a pre-formatted string: {message}"
3383+
);
3384+
assert!(
3385+
message.contains("stacker secrets set npm_credentials"),
3386+
"Message should include the remediation command: {message}"
3387+
);
3388+
}
3389+
3390+
#[test]
3391+
fn agent_command_error_message_exposes_vault_path_in_debug_mode_for_npm_credentials_invalid() {
3392+
let _guard = npm_creds_env_lock();
3393+
std::env::set_var("STACKER_DEBUG", "1");
3394+
3395+
let vault_path = "secret/base/status_panel/hosts/86/npm_credentials";
3396+
// Test both paths in debug mode
3397+
let msg_via_result =
3398+
agent_command_error_message(&npm_creds_invalid_info_via_result(vault_path));
3399+
let msg_via_error =
3400+
agent_command_error_message(&npm_creds_invalid_info_via_error(vault_path));
3401+
std::env::remove_var("STACKER_DEBUG");
3402+
3403+
let msg_via_result = msg_via_result.expect("error message (result path)");
3404+
assert!(
3405+
msg_via_result.contains(vault_path),
3406+
"Vault path should appear in debug output (result path): {msg_via_result}"
3407+
);
3408+
assert!(
3409+
msg_via_result.contains("stacker secrets set npm_credentials"),
3410+
"Debug output should still include the remediation command: {msg_via_result}"
3411+
);
3412+
3413+
let msg_via_error = msg_via_error.expect("error message (error field path)");
3414+
assert!(
3415+
msg_via_error.contains(vault_path),
3416+
"Vault path should appear in debug output (error field path): {msg_via_error}"
3417+
);
3418+
assert!(
3419+
msg_via_error.contains("stacker secrets set npm_credentials"),
3420+
"Debug output should still include the remediation command: {msg_via_error}"
3421+
);
3422+
}
3423+
32303424
#[test]
32313425
fn agent_command_error_message_adds_proxy_route_diagnostics_for_npm_create_failed() {
32323426
let info = AgentCommandInfo {

src/console/commands/cli/cloud_firewall.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::cli::deployment_lock::DeploymentLock;
12
use crate::cli::error::CliError;
23
use crate::cli::runtime::CliRuntime;
34
use crate::console::commands::CallableTrait;
@@ -40,6 +41,16 @@ impl CloudFirewallCommand {
4041
}
4142

4243
let project_dir = std::env::current_dir().map_err(CliError::Io)?;
44+
45+
if let Ok(Some(lock)) = DeploymentLock::load_active(&project_dir) {
46+
if let Some(server_name) = lock.server_name {
47+
if let Ok(Some(server)) =
48+
ctx.block_on(ctx.client.find_server_by_name(&server_name))
49+
{
50+
return Ok(server.id);
51+
}
52+
}
53+
}
4354
let config_path = project_dir.join("stacker.yml");
4455
if !config_path.exists() {
4556
return Err(CliError::ConfigValidation(

0 commit comments

Comments
 (0)