Skip to content

Commit ddf308b

Browse files
Sbussisoclaude
andcommitted
fix(dashboard): accept bare /wipe /wipe as confirmation
A second bare `/wipe` within 30s of the first now confirms the wipe, matching the intuitive CLI pattern the prompt implied. Previously the handler required the literal arg string `confirm` (i.e. `/wipe confirm`), so pressing /wipe twice silently re-showed the warning — the node kept running and nothing happened. Implemented as a generic confirm-on-repeat helper (`check_or_arm_confirm`) also applied to `/reauth`. Only bare repeats count, so a typo like `/wipe dry-run` cannot accidentally trigger destruction. Any unrelated command clears the pending slot, and the 30-second timeout prevents a forgotten terminal from confirming hours later. The explicit `/wipe confirm` form still works for scriptability. Prompts updated to: "Press /wipe again within 30s (or type /wipe confirm) to proceed." Added 7 unit tests covering arm / confirm / non-bare-typo / cross-command / timeout / clear paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 059fbb1 commit ddf308b

1 file changed

Lines changed: 173 additions & 4 deletions

File tree

src/dashboard.rs

Lines changed: 173 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,21 @@ pub struct DashboardState {
197197
/// API client (for /wipe backend decommission). Optional because
198198
/// dashboards created in test-mode / `run_once` don't have one.
199199
pub api_client: Option<ApiClient>,
200+
/// Armed destructive-command confirmation. Stores `(command, armed_at)`.
201+
/// Running the same command again without args within the timeout
202+
/// window (see [`CONFIRM_TIMEOUT`]) counts as confirmation. Cleared
203+
/// as soon as *any* other command is dispatched, so the user can't
204+
/// accidentally confirm a pending wipe by typing /wipe minutes later
205+
/// after having done other things in between.
206+
pub pending_confirm: Option<(String, Instant)>,
200207
}
201208

209+
/// How long after the first bare `/wipe` or `/reauth` a repeat press
210+
/// still counts as confirmation. 30s is plenty of time to read the
211+
/// warning and decide, but short enough that a forgotten terminal left
212+
/// on the settings page won't accept a confirmation hours later.
213+
pub const CONFIRM_TIMEOUT: Duration = Duration::from_secs(30);
214+
202215
impl DashboardState {
203216
pub fn new(node_id: impl Into<String>, api_url: impl Into<String>) -> Self {
204217
Self {
@@ -218,6 +231,7 @@ impl DashboardState {
218231
db: None,
219232
hls_dir: None,
220233
api_client: None,
234+
pending_confirm: None,
221235
}
222236
}
223237

@@ -975,6 +989,53 @@ impl Dashboard {
975989
}
976990
}
977991

992+
/// Check whether a destructive command should run, using either
993+
/// an explicit `/<cmd> confirm` argument OR a repeat *bare* press
994+
/// of the same command within [`CONFIRM_TIMEOUT`] of the first.
995+
///
996+
/// - `explicit_arg` — true iff the user typed `/<cmd> confirm`.
997+
/// - `bare` — true iff the user typed `/<cmd>` with no args.
998+
///
999+
/// Only bare repeats count as confirmation. A call with unrecognized
1000+
/// arguments (e.g. `/wipe dry-run`) re-arms the prompt but does not
1001+
/// consume a pending confirmation, so a previous `/wipe` doesn't get
1002+
/// turned into destruction by a typo.
1003+
///
1004+
/// Returns `true` if the command should proceed now; in that case
1005+
/// the pending-confirm slot is cleared. Returns `false` if the
1006+
/// caller should show the warning prompt; in that case the slot is
1007+
/// (re-)armed so the next bare press of the same command confirms.
1008+
fn check_or_arm_confirm(&self, cmd: &str, explicit_arg: bool, bare: bool) -> bool {
1009+
let Ok(mut s) = self.0.lock() else {
1010+
// Lock poisoned — fail closed (require re-arming).
1011+
return false;
1012+
};
1013+
1014+
let repeat_confirm = bare
1015+
&& matches!(
1016+
&s.pending_confirm,
1017+
Some((pending_cmd, armed_at))
1018+
if pending_cmd == cmd && armed_at.elapsed() < CONFIRM_TIMEOUT
1019+
);
1020+
1021+
if explicit_arg || repeat_confirm {
1022+
s.pending_confirm = None;
1023+
true
1024+
} else {
1025+
s.pending_confirm = Some((cmd.to_string(), Instant::now()));
1026+
false
1027+
}
1028+
}
1029+
1030+
/// Discard any pending destructive-command confirmation. Called
1031+
/// whenever the user dispatches an unrelated command, so they can't
1032+
/// accidentally confirm a stale `/wipe` hours later.
1033+
fn clear_pending_confirm(&self) {
1034+
if let Ok(mut s) = self.0.lock() {
1035+
s.pending_confirm = None;
1036+
}
1037+
}
1038+
9781039
/// Parse and execute a slash command from the input bar.
9791040
fn execute_command(&self, input: &str, stop: &Arc<std::sync::atomic::AtomicBool>) {
9801041
let input = input.trim();
@@ -988,6 +1049,13 @@ impl Dashboard {
9881049
let cmd = parts.first().copied().unwrap_or("");
9891050
let args = if parts.len() > 1 { &parts[1..] } else { &parts[..0] };
9901051

1052+
// Any command other than the pending destructive one invalidates
1053+
// the armed confirmation. The handlers for /wipe and /reauth
1054+
// below re-arm or consume it as appropriate.
1055+
if !matches!(cmd, "wipe" | "reauth") {
1056+
self.clear_pending_confirm();
1057+
}
1058+
9911059
// Check current view for settings-only commands
9921060
let on_settings = self.0.lock().map(|s| s.current_view == View::Settings).unwrap_or(false);
9931061

@@ -1163,7 +1231,9 @@ impl Dashboard {
11631231
self.log_info(format!("Logs exported to {}", filename));
11641232
}
11651233
"wipe" if on_settings => {
1166-
let confirm = args.first().copied() == Some("confirm");
1234+
let explicit_arg = args.first().copied() == Some("confirm");
1235+
let bare = args.is_empty();
1236+
let confirm = self.check_or_arm_confirm("wipe", explicit_arg, bare);
11671237
if !confirm {
11681238
self.set_output(vec![
11691239
"This will permanently delete ALL data and unpair from Command Center:"
@@ -1175,7 +1245,8 @@ impl Dashboard {
11751245
"The node will shut down. Run setup again with a NEW".to_string(),
11761246
"node ID / API key from Command Center to re-pair.".to_string(),
11771247
String::new(),
1178-
"Type /wipe confirm to proceed.".to_string(),
1248+
"Press /wipe again within 30s (or type /wipe confirm) to proceed."
1249+
.to_string(),
11791250
]);
11801251
} else {
11811252
// Snapshot what we need out of the lock before doing
@@ -1257,13 +1328,16 @@ impl Dashboard {
12571328
}
12581329
}
12591330
"reauth" if on_settings => {
1260-
let confirm = args.first().copied() == Some("confirm");
1331+
let explicit_arg = args.first().copied() == Some("confirm");
1332+
let bare = args.is_empty();
1333+
let confirm = self.check_or_arm_confirm("reauth", explicit_arg, bare);
12611334
if !confirm {
12621335
self.set_output(vec![
12631336
"This will clear your credentials and stop the node.".to_string(),
12641337
"You will need to run setup again with new credentials.".to_string(),
12651338
String::new(),
1266-
"Type /reauth confirm to proceed.".to_string(),
1339+
"Press /reauth again within 30s (or type /reauth confirm) to proceed."
1340+
.to_string(),
12671341
]);
12681342
} else {
12691343
if let Ok(s) = self.0.lock() {
@@ -1465,3 +1539,98 @@ fn truncate_ansi(s: &str, max: usize) -> String {
14651539

14661540
result
14671541
}
1542+
1543+
// ─── Tests ────────────────────────────────────────────────────────────────────
1544+
1545+
#[cfg(test)]
1546+
mod tests {
1547+
use super::*;
1548+
1549+
/// Build a fresh Dashboard with no DB / API / HLS dir attached —
1550+
/// enough to exercise the pure-state confirm helpers.
1551+
fn fresh() -> Dashboard {
1552+
Dashboard::new("test-node", "http://test")
1553+
}
1554+
1555+
fn pending_cmd(dash: &Dashboard) -> Option<String> {
1556+
dash.0
1557+
.lock()
1558+
.ok()
1559+
.and_then(|s| s.pending_confirm.as_ref().map(|(c, _)| c.clone()))
1560+
}
1561+
1562+
#[test]
1563+
fn first_bare_press_arms_but_does_not_confirm() {
1564+
let d = fresh();
1565+
let confirmed = d.check_or_arm_confirm("wipe", /*explicit*/ false, /*bare*/ true);
1566+
assert!(!confirmed, "first press must not proceed");
1567+
assert_eq!(pending_cmd(&d).as_deref(), Some("wipe"));
1568+
}
1569+
1570+
#[test]
1571+
fn second_bare_press_within_timeout_confirms() {
1572+
let d = fresh();
1573+
assert!(!d.check_or_arm_confirm("wipe", false, true));
1574+
let confirmed = d.check_or_arm_confirm("wipe", false, true);
1575+
assert!(confirmed, "second bare press must confirm");
1576+
assert_eq!(pending_cmd(&d), None, "pending cleared after confirm");
1577+
}
1578+
1579+
#[test]
1580+
fn explicit_confirm_arg_always_proceeds() {
1581+
let d = fresh();
1582+
let confirmed = d.check_or_arm_confirm("wipe", /*explicit*/ true, /*bare*/ false);
1583+
assert!(confirmed);
1584+
assert_eq!(pending_cmd(&d), None);
1585+
}
1586+
1587+
#[test]
1588+
fn second_press_with_unknown_arg_does_not_confirm() {
1589+
// /wipe then /wipe dry-run should NOT wipe — only bare repeat counts.
1590+
let d = fresh();
1591+
assert!(!d.check_or_arm_confirm("wipe", false, true)); // arm
1592+
let confirmed = d.check_or_arm_confirm("wipe", false, false); // non-bare
1593+
assert!(!confirmed, "non-bare repeat must not confirm");
1594+
// And it re-arms rather than leaving stale state.
1595+
assert_eq!(pending_cmd(&d).as_deref(), Some("wipe"));
1596+
}
1597+
1598+
#[test]
1599+
fn pending_for_different_command_does_not_cross_confirm() {
1600+
// Arming /wipe must not let a bare /reauth sneak through.
1601+
let d = fresh();
1602+
assert!(!d.check_or_arm_confirm("wipe", false, true));
1603+
let confirmed = d.check_or_arm_confirm("reauth", false, true);
1604+
assert!(!confirmed, "different pending cmd must not confirm");
1605+
assert_eq!(pending_cmd(&d).as_deref(), Some("reauth"));
1606+
}
1607+
1608+
#[test]
1609+
fn clear_pending_confirm_drops_armed_state() {
1610+
let d = fresh();
1611+
assert!(!d.check_or_arm_confirm("wipe", false, true));
1612+
d.clear_pending_confirm();
1613+
assert_eq!(pending_cmd(&d), None);
1614+
// After clear, a fresh bare press must re-arm, not confirm.
1615+
let confirmed = d.check_or_arm_confirm("wipe", false, true);
1616+
assert!(!confirmed);
1617+
}
1618+
1619+
#[test]
1620+
fn expired_pending_requires_rearming() {
1621+
// Simulate a stale arming older than CONFIRM_TIMEOUT by stuffing
1622+
// an `Instant` from far in the past into the slot directly.
1623+
let d = fresh();
1624+
{
1625+
let mut s = d.0.lock().unwrap();
1626+
let old = Instant::now()
1627+
.checked_sub(CONFIRM_TIMEOUT + Duration::from_secs(1))
1628+
.expect("system clock must support subtraction");
1629+
s.pending_confirm = Some(("wipe".to_string(), old));
1630+
}
1631+
let confirmed = d.check_or_arm_confirm("wipe", false, true);
1632+
assert!(!confirmed, "expired pending must require re-arming");
1633+
// And the slot should be re-armed with a fresh timestamp.
1634+
assert_eq!(pending_cmd(&d).as_deref(), Some("wipe"));
1635+
}
1636+
}

0 commit comments

Comments
 (0)