Update public CLI confirmation gates
This commit is contained in:
parent
66fec04e75
commit
7a80d87376
4 changed files with 250 additions and 5 deletions
|
|
@ -1,7 +1,7 @@
|
||||||
{
|
{
|
||||||
"kind": "disasmer-filtered-public-tree",
|
"kind": "disasmer-filtered-public-tree",
|
||||||
"source_commit": "69c1074abe5cb9ccb3f31ab39327bfcbfd7b74b3",
|
"source_commit": "611a91087c5a78ed23617674f2664f7cf07af020",
|
||||||
"release_name": "dryrun-69c1074abe5c",
|
"release_name": "dryrun-611a91087c5a",
|
||||||
"filtered_out": [
|
"filtered_out": [
|
||||||
"private/**",
|
"private/**",
|
||||||
"experiments/**",
|
"experiments/**",
|
||||||
|
|
|
||||||
|
|
@ -937,8 +937,51 @@ fn non_interactive_browser_login_report(args: &LoginArgs) -> Value {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn confirmation_required_report(
|
||||||
|
command: &str,
|
||||||
|
operation: &str,
|
||||||
|
target: Value,
|
||||||
|
confirm_command: String,
|
||||||
|
) -> Value {
|
||||||
|
let message = format!("{command} requires --yes before {operation}");
|
||||||
|
let next_actions = json!([
|
||||||
|
confirm_command,
|
||||||
|
"review the command target before confirming",
|
||||||
|
"rerun with --json to inspect the safe failure"
|
||||||
|
]);
|
||||||
|
let mut machine_error = cli_error_summary_for_category("policy", &message);
|
||||||
|
if let Some(object) = machine_error.as_object_mut() {
|
||||||
|
object.insert("confirmation_required".to_owned(), json!(true));
|
||||||
|
object.insert("next_actions".to_owned(), next_actions.clone());
|
||||||
|
}
|
||||||
|
json!({
|
||||||
|
"command": command,
|
||||||
|
"status": "confirmation_required",
|
||||||
|
"operation": operation,
|
||||||
|
"target": target,
|
||||||
|
"requires_confirmation": true,
|
||||||
|
"confirmation_required": true,
|
||||||
|
"explicit_user_action_required": true,
|
||||||
|
"coordinator_request_sent": false,
|
||||||
|
"safe_failure": true,
|
||||||
|
"next_actions": next_actions,
|
||||||
|
"machine_error": machine_error,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
fn logout_report(args: AuthLogoutArgs, cwd: PathBuf, command: &str) -> Result<Value> {
|
fn logout_report(args: AuthLogoutArgs, cwd: PathBuf, command: &str) -> Result<Value> {
|
||||||
let session_file = session_config_file(&cwd);
|
let session_file = session_config_file(&cwd);
|
||||||
|
if !args.yes {
|
||||||
|
return Ok(confirmation_required_report(
|
||||||
|
command,
|
||||||
|
"delete_cli_session",
|
||||||
|
json!({
|
||||||
|
"session_file": session_file,
|
||||||
|
"node_credentials_untouched": true,
|
||||||
|
}),
|
||||||
|
format!("disasmer {command} --yes"),
|
||||||
|
));
|
||||||
|
}
|
||||||
let existed = session_file.exists();
|
let existed = session_file.exists();
|
||||||
if existed {
|
if existed {
|
||||||
std::fs::remove_file(&session_file)
|
std::fs::remove_file(&session_file)
|
||||||
|
|
@ -1066,6 +1109,20 @@ fn key_list_report(args: KeyListArgs) -> Result<Value> {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn key_revoke_report(args: KeyRevokeArgs) -> Result<Value> {
|
fn key_revoke_report(args: KeyRevokeArgs) -> Result<Value> {
|
||||||
|
if !args.yes {
|
||||||
|
return Ok(confirmation_required_report(
|
||||||
|
"key revoke",
|
||||||
|
"revoke_agent_public_key",
|
||||||
|
json!({
|
||||||
|
"coordinator": args.scope.coordinator,
|
||||||
|
"tenant": args.scope.tenant,
|
||||||
|
"project": args.scope.project,
|
||||||
|
"user": args.scope.user,
|
||||||
|
"agent": args.agent,
|
||||||
|
}),
|
||||||
|
format!("disasmer key revoke --agent {} --yes", args.agent),
|
||||||
|
));
|
||||||
|
}
|
||||||
if let Some(coordinator) = &args.scope.coordinator {
|
if let Some(coordinator) = &args.scope.coordinator {
|
||||||
let tenant = args.scope.tenant.clone();
|
let tenant = args.scope.tenant.clone();
|
||||||
let project = args.scope.project.clone();
|
let project = args.scope.project.clone();
|
||||||
|
|
@ -1465,6 +1522,20 @@ fn node_descriptors_report(
|
||||||
}
|
}
|
||||||
|
|
||||||
fn node_revoke_report(args: NodeRevokeArgs) -> Result<Value> {
|
fn node_revoke_report(args: NodeRevokeArgs) -> Result<Value> {
|
||||||
|
if !args.yes {
|
||||||
|
return Ok(confirmation_required_report(
|
||||||
|
"node revoke",
|
||||||
|
"revoke_node_credential",
|
||||||
|
json!({
|
||||||
|
"coordinator": args.scope.coordinator,
|
||||||
|
"tenant": args.scope.tenant,
|
||||||
|
"project": args.scope.project,
|
||||||
|
"user": args.scope.user,
|
||||||
|
"node": args.node,
|
||||||
|
}),
|
||||||
|
format!("disasmer node revoke --node {} --yes", args.node),
|
||||||
|
));
|
||||||
|
}
|
||||||
if let Some(coordinator) = &args.scope.coordinator {
|
if let Some(coordinator) = &args.scope.coordinator {
|
||||||
let tenant = args.scope.tenant.clone();
|
let tenant = args.scope.tenant.clone();
|
||||||
let project = args.scope.project.clone();
|
let project = args.scope.project.clone();
|
||||||
|
|
@ -1533,6 +1604,19 @@ fn process_status_report(args: ProcessStatusArgs) -> Result<Value> {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn process_restart_report(args: ProcessRestartArgs) -> Result<Value> {
|
fn process_restart_report(args: ProcessRestartArgs) -> Result<Value> {
|
||||||
|
if !args.yes {
|
||||||
|
return Ok(confirmation_required_report(
|
||||||
|
"process restart",
|
||||||
|
"restart_virtual_process",
|
||||||
|
json!({
|
||||||
|
"coordinator": args.scope.coordinator,
|
||||||
|
"tenant": args.scope.tenant,
|
||||||
|
"project": args.scope.project,
|
||||||
|
"process": args.process,
|
||||||
|
}),
|
||||||
|
format!("disasmer process restart --process {} --yes", args.process),
|
||||||
|
));
|
||||||
|
}
|
||||||
if let Some(coordinator) = &args.scope.coordinator {
|
if let Some(coordinator) = &args.scope.coordinator {
|
||||||
let mut session = JsonLineSession::connect(coordinator)?;
|
let mut session = JsonLineSession::connect(coordinator)?;
|
||||||
let response = session.request(json!({
|
let response = session.request(json!({
|
||||||
|
|
@ -1567,6 +1651,21 @@ fn process_restart_report(args: ProcessRestartArgs) -> Result<Value> {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn process_cancel_report(args: ProcessCancelArgs) -> Result<Value> {
|
fn process_cancel_report(args: ProcessCancelArgs) -> Result<Value> {
|
||||||
|
if !args.yes {
|
||||||
|
return Ok(confirmation_required_report(
|
||||||
|
"process cancel",
|
||||||
|
"cancel_virtual_process",
|
||||||
|
json!({
|
||||||
|
"coordinator": args.scope.coordinator,
|
||||||
|
"tenant": args.scope.tenant,
|
||||||
|
"project": args.scope.project,
|
||||||
|
"process": args.process,
|
||||||
|
"node": args.node,
|
||||||
|
"task": args.task,
|
||||||
|
}),
|
||||||
|
format!("disasmer process cancel --process {} --yes", args.process),
|
||||||
|
));
|
||||||
|
}
|
||||||
if let Some(coordinator) = &args.scope.coordinator {
|
if let Some(coordinator) = &args.scope.coordinator {
|
||||||
let mut session = JsonLineSession::connect(coordinator)?;
|
let mut session = JsonLineSession::connect(coordinator)?;
|
||||||
let response = session.request(json!({
|
let response = session.request(json!({
|
||||||
|
|
@ -1621,6 +1720,23 @@ fn task_list_report(args: TaskListArgs) -> Result<Value> {
|
||||||
}
|
}
|
||||||
|
|
||||||
fn task_restart_report(args: TaskRestartArgs) -> Result<Value> {
|
fn task_restart_report(args: TaskRestartArgs) -> Result<Value> {
|
||||||
|
if !args.yes {
|
||||||
|
return Ok(confirmation_required_report(
|
||||||
|
"task restart",
|
||||||
|
"restart_selected_task",
|
||||||
|
json!({
|
||||||
|
"coordinator": args.scope.coordinator,
|
||||||
|
"tenant": args.scope.tenant,
|
||||||
|
"project": args.scope.project,
|
||||||
|
"process": args.process,
|
||||||
|
"task": args.task,
|
||||||
|
}),
|
||||||
|
format!(
|
||||||
|
"disasmer task restart {} --process {} --yes",
|
||||||
|
args.task, args.process
|
||||||
|
),
|
||||||
|
));
|
||||||
|
}
|
||||||
if let Some(coordinator) = &args.scope.coordinator {
|
if let Some(coordinator) = &args.scope.coordinator {
|
||||||
let mut session = JsonLineSession::connect(coordinator)?;
|
let mut session = JsonLineSession::connect(coordinator)?;
|
||||||
let response = session.request_allow_error(json!({
|
let response = session.request_allow_error(json!({
|
||||||
|
|
@ -2082,6 +2198,19 @@ fn admin_suspend_tenant_report(args: AdminSuspendTenantArgs) -> Result<Value> {
|
||||||
let tenant = args
|
let tenant = args
|
||||||
.target_tenant
|
.target_tenant
|
||||||
.unwrap_or_else(|| args.scope.tenant.clone());
|
.unwrap_or_else(|| args.scope.tenant.clone());
|
||||||
|
if !args.yes {
|
||||||
|
return Ok(confirmation_required_report(
|
||||||
|
"admin suspend-tenant",
|
||||||
|
"suspend_tenant",
|
||||||
|
json!({
|
||||||
|
"coordinator": args.scope.coordinator,
|
||||||
|
"actor_tenant": args.scope.tenant,
|
||||||
|
"actor_user": args.scope.user,
|
||||||
|
"target_tenant": tenant,
|
||||||
|
}),
|
||||||
|
"disasmer admin suspend-tenant --yes".to_owned(),
|
||||||
|
));
|
||||||
|
}
|
||||||
if let Some(coordinator) = &args.scope.coordinator {
|
if let Some(coordinator) = &args.scope.coordinator {
|
||||||
let actor_tenant = args.scope.tenant.clone();
|
let actor_tenant = args.scope.tenant.clone();
|
||||||
let actor_user = args.scope.user.clone();
|
let actor_user = args.scope.user.clone();
|
||||||
|
|
@ -6779,6 +6908,27 @@ mod tests {
|
||||||
fs::create_dir_all(session_file.parent().unwrap()).unwrap();
|
fs::create_dir_all(session_file.parent().unwrap()).unwrap();
|
||||||
fs::write(&session_file, br#"{"kind":"human","token":"local"}"#).unwrap();
|
fs::write(&session_file, br#"{"kind":"human","token":"local"}"#).unwrap();
|
||||||
|
|
||||||
|
let unconfirmed = logout_report(
|
||||||
|
AuthLogoutArgs {
|
||||||
|
yes: false,
|
||||||
|
scope: CliScopeArgs {
|
||||||
|
coordinator: None,
|
||||||
|
tenant: "tenant".to_owned(),
|
||||||
|
project: "project".to_owned(),
|
||||||
|
user: "user".to_owned(),
|
||||||
|
json: false,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
temp.path().to_path_buf(),
|
||||||
|
"logout",
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(unconfirmed["status"], "confirmation_required");
|
||||||
|
assert_eq!(unconfirmed["coordinator_request_sent"], false);
|
||||||
|
assert_eq!(unconfirmed["machine_error"]["category"], "policy");
|
||||||
|
assert!(session_file.exists());
|
||||||
|
|
||||||
let report = logout_report(
|
let report = logout_report(
|
||||||
AuthLogoutArgs {
|
AuthLogoutArgs {
|
||||||
yes: true,
|
yes: true,
|
||||||
|
|
@ -6802,6 +6952,72 @@ mod tests {
|
||||||
assert!(!session_file.exists());
|
assert!(!session_file.exists());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn mutating_commands_require_yes_before_side_effects() {
|
||||||
|
let scope = CliScopeArgs {
|
||||||
|
coordinator: Some("127.0.0.1:9".to_owned()),
|
||||||
|
tenant: "tenant".to_owned(),
|
||||||
|
project: "project".to_owned(),
|
||||||
|
user: "user".to_owned(),
|
||||||
|
json: false,
|
||||||
|
};
|
||||||
|
let reports = [
|
||||||
|
key_revoke_report(KeyRevokeArgs {
|
||||||
|
scope: scope.clone(),
|
||||||
|
agent: "agent-ci".to_owned(),
|
||||||
|
yes: false,
|
||||||
|
})
|
||||||
|
.unwrap(),
|
||||||
|
node_revoke_report(NodeRevokeArgs {
|
||||||
|
scope: scope.clone(),
|
||||||
|
node: "node-a".to_owned(),
|
||||||
|
yes: false,
|
||||||
|
})
|
||||||
|
.unwrap(),
|
||||||
|
process_restart_report(ProcessRestartArgs {
|
||||||
|
scope: scope.clone(),
|
||||||
|
process: "vp".to_owned(),
|
||||||
|
yes: false,
|
||||||
|
})
|
||||||
|
.unwrap(),
|
||||||
|
process_cancel_report(ProcessCancelArgs {
|
||||||
|
scope: scope.clone(),
|
||||||
|
process: "vp".to_owned(),
|
||||||
|
node: None,
|
||||||
|
task: None,
|
||||||
|
yes: false,
|
||||||
|
})
|
||||||
|
.unwrap(),
|
||||||
|
task_restart_report(TaskRestartArgs {
|
||||||
|
scope: scope.clone(),
|
||||||
|
task: "compile-linux".to_owned(),
|
||||||
|
process: "vp".to_owned(),
|
||||||
|
yes: false,
|
||||||
|
})
|
||||||
|
.unwrap(),
|
||||||
|
admin_suspend_tenant_report(AdminSuspendTenantArgs {
|
||||||
|
scope,
|
||||||
|
target_tenant: Some("tenant".to_owned()),
|
||||||
|
yes: false,
|
||||||
|
})
|
||||||
|
.unwrap(),
|
||||||
|
];
|
||||||
|
|
||||||
|
for report in reports {
|
||||||
|
assert_eq!(report["status"], "confirmation_required");
|
||||||
|
assert_eq!(report["requires_confirmation"], true);
|
||||||
|
assert_eq!(report["coordinator_request_sent"], false);
|
||||||
|
assert_eq!(report["safe_failure"], true);
|
||||||
|
assert_eq!(report["machine_error"]["category"], "policy");
|
||||||
|
assert_eq!(report["machine_error"]["confirmation_required"], true);
|
||||||
|
assert!(report["next_actions"]
|
||||||
|
.as_array()
|
||||||
|
.unwrap()
|
||||||
|
.iter()
|
||||||
|
.any(|action| action.as_str().unwrap_or_default().contains("--yes")));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn cli_first_json_mode_parses_for_primary_commands() {
|
fn cli_first_json_mode_parses_for_primary_commands() {
|
||||||
for args in [
|
for args in [
|
||||||
|
|
@ -7771,7 +7987,7 @@ mod tests {
|
||||||
process: "vp".to_owned(),
|
process: "vp".to_owned(),
|
||||||
node: None,
|
node: None,
|
||||||
task: None,
|
task: None,
|
||||||
yes: false,
|
yes: true,
|
||||||
})
|
})
|
||||||
.unwrap();
|
.unwrap();
|
||||||
server.join().unwrap();
|
server.join().unwrap();
|
||||||
|
|
@ -7803,7 +8019,7 @@ mod tests {
|
||||||
"compile-linux"
|
"compile-linux"
|
||||||
);
|
);
|
||||||
assert_eq!(cancel["cancel_request"]["affected_nodes"][1], "node-b");
|
assert_eq!(cancel["cancel_request"]["affected_nodes"][1], "node-b");
|
||||||
assert_eq!(cancel["cancel_request"]["requires_confirmation"], true);
|
assert_eq!(cancel["cancel_request"]["requires_confirmation"], false);
|
||||||
assert_eq!(cancel["cancel_request"]["website_required"], false);
|
assert_eq!(cancel["cancel_request"]["website_required"], false);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
cancel["cancel_request"]["whole_process_cancel_available"],
|
cancel["cancel_request"]["whole_process_cancel_available"],
|
||||||
|
|
@ -8100,7 +8316,8 @@ mod tests {
|
||||||
yes: false,
|
yes: false,
|
||||||
})
|
})
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(cancel["status"], "requires_coordinator");
|
assert_eq!(cancel["status"], "confirmation_required");
|
||||||
assert_eq!(cancel["requires_confirmation"], true);
|
assert_eq!(cancel["requires_confirmation"], true);
|
||||||
|
assert_eq!(cancel["coordinator_request_sent"], false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -108,6 +108,27 @@ async function main() {
|
||||||
assert(
|
assert(
|
||||||
report.run_start.machine_error.next_actions.includes("disasmer quota status")
|
report.run_start.machine_error.next_actions.includes("disasmer quota status")
|
||||||
);
|
);
|
||||||
|
|
||||||
|
const confirmation = await runDisasmer([
|
||||||
|
"process",
|
||||||
|
"cancel",
|
||||||
|
"--coordinator",
|
||||||
|
"127.0.0.1:9",
|
||||||
|
"--json",
|
||||||
|
]);
|
||||||
|
assert.strictEqual(confirmation.signal, null, confirmation.stderr);
|
||||||
|
assert.strictEqual(confirmation.code, 23, confirmation.stderr);
|
||||||
|
const confirmationReport = JSON.parse(confirmation.stdout);
|
||||||
|
assert.strictEqual(confirmationReport.status, "confirmation_required");
|
||||||
|
assert.strictEqual(confirmationReport.coordinator_request_sent, false);
|
||||||
|
assert.strictEqual(confirmationReport.machine_error.category, "policy");
|
||||||
|
assert.strictEqual(
|
||||||
|
confirmationReport.machine_error.process_exit_code_applied,
|
||||||
|
true
|
||||||
|
);
|
||||||
|
assert(
|
||||||
|
confirmationReport.next_actions.some((action) => action.includes("--yes"))
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
main()
|
main()
|
||||||
|
|
|
||||||
|
|
@ -92,6 +92,11 @@ expect(
|
||||||
"locality failure safe guidance",
|
"locality failure safe guidance",
|
||||||
/If direct transfer or locality assumptions fail[\s\S]*connectivity-category safe failures[\s\S]*coordinator bulk relay was not used/
|
/If direct transfer or locality assumptions fail[\s\S]*connectivity-category safe failures[\s\S]*coordinator bulk relay was not used/
|
||||||
);
|
);
|
||||||
|
expect(
|
||||||
|
criteria,
|
||||||
|
"mutating commands require confirmation",
|
||||||
|
/Mutating or dangerous commands support `--yes`[\s\S]*confirmation-required safe failure[\s\S]*do not send coordinator requests/
|
||||||
|
);
|
||||||
expect(
|
expect(
|
||||||
cliFirstAcceptance,
|
cliFirstAcceptance,
|
||||||
"CLI-first acceptance report",
|
"CLI-first acceptance report",
|
||||||
|
|
@ -202,6 +207,8 @@ for (const [name, pattern] of [
|
||||||
["CLI error classifier coverage", /fn cli_error_classifier_distinguishes_mvp_failure_categories\(\)/],
|
["CLI error classifier coverage", /fn cli_error_classifier_distinguishes_mvp_failure_categories\(\)/],
|
||||||
["CLI command exit-code coverage", /fn command_report_exit_code_marks_command_failures_only\(\)/],
|
["CLI command exit-code coverage", /fn command_report_exit_code_marks_command_failures_only\(\)/],
|
||||||
["CLI top-level logout alias coverage", /fn top_level_logout_alias_removes_only_cli_session_state\(\)/],
|
["CLI top-level logout alias coverage", /fn top_level_logout_alias_removes_only_cli_session_state\(\)/],
|
||||||
|
["CLI confirmation gate helper", /fn confirmation_required_report[\s\S]*coordinator_request_sent[\s\S]*confirmation_required/],
|
||||||
|
["CLI mutating confirmation coverage", /fn mutating_commands_require_yes_before_side_effects\(\)/],
|
||||||
["CLI run rejection category coverage", /fn run_rejection_reports_machine_readable_error_category\(\)/],
|
["CLI run rejection category coverage", /fn run_rejection_reports_machine_readable_error_category\(\)/],
|
||||||
["CLI locality failure classifier", /fn classify_cli_error_message\(message: &str\)[\s\S]*message_mentions_locality_failure\(&message\)[\s\S]*return "connectivity"/],
|
["CLI locality failure classifier", /fn classify_cli_error_message\(message: &str\)[\s\S]*message_mentions_locality_failure\(&message\)[\s\S]*return "connectivity"/],
|
||||||
["CLI locality failure report helper", /fn task_locality_failure_from_reason\(reason: &Value\) -> Value[\s\S]*coordinator_bulk_relay_used[\s\S]*safe_next_actions/],
|
["CLI locality failure report helper", /fn task_locality_failure_from_reason\(reason: &Value\) -> Value[\s\S]*coordinator_bulk_relay_used[\s\S]*safe_next_actions/],
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue