Fix subnets validation

This commit is contained in:
2026-10-26 22:59:11 +02:00
parent e141c03c2c
commit ecc5c83d78
11 changed files with 605 additions and 68 deletions
+20
View File
@@ -46,10 +46,29 @@ pub struct ClientDisplay {
pub identity: Option<PublicKey>,
pub label: String,
pub allowed_ips: Vec<String>,
pub has_subnet_routes: bool,
pub issues: Vec<crate::wireguard::PeerIssue>,
pub mutable_identity: Option<PublicKey>,
}
impl ClientDisplay {
/// Whether a retained parser issue describes a valid address shape the
/// client model cannot manage, rather than a malformed configuration.
pub fn is_capability_limitation_issue(&self, issue: &crate::wireguard::PeerIssue) -> bool {
matches!(
issue,
crate::wireguard::PeerIssue::UnsupportedAllowedIpShape { .. }
| crate::wireguard::PeerIssue::MissingIpv4HostRoute
)
}
pub fn presented_issues(&self) -> impl Iterator<Item = &crate::wireguard::PeerIssue> {
self.issues
.iter()
.filter(|issue| !self.is_capability_limitation_issue(issue))
}
}
/// Secret-free display projection of the current document and client state.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct SessionDisplay {
@@ -553,6 +572,7 @@ impl Application {
identity,
label,
allowed_ips: peer.allowed_ips.iter().map(ToString::to_string).collect(),
has_subnet_routes: peer.has_subnet_routes,
issues: peer.issues.clone(),
mutable_identity: projected
.filter(|client| self.states.draft().get(&client.identity).is_some())
+6
View File
@@ -84,6 +84,12 @@ fn encode_bytes(bytes: &[u8]) -> Result<QrCodeOutput, QrError> {
Ok(QrCodeOutput { side, modules })
}
/// Small, non-secret QR used only to exercise the terminal disclosure renderer.
#[cfg(test)]
pub(crate) fn synthetic_test_output() -> QrCodeOutput {
encode_bytes(b"synthetic QR disclosure fixture").expect("synthetic QR payload encodes")
}
#[cfg(test)]
mod tests {
use super::*;
+229 -18
View File
@@ -230,17 +230,16 @@ fn keyboard_hints(state: &UiState, forms: &FormController) -> Vec<Hint> {
add("c", "Commit", HintKind::Action);
add("x", "discard", HintKind::Destructive);
add("R", "restart", HintKind::Destructive);
if let Some(id) = state.selected_identity {
if state
if let Some(id) = state.selected_identity
&& state
.view
.as_ref()
.and_then(|view| view.client_states.get(&id))
.is_some_and(|row| row.saved.is_some())
&& selected
{
add("o", "first file", HintKind::Action);
add("O", "password reissue", HintKind::Action);
}
&& selected
{
add("o", "first file", HintKind::Action);
add("O", "password reissue", HintKind::Action);
}
add("b", "backups / restore", HintKind::Action);
add("B", "backup", HintKind::Action);
@@ -497,6 +496,7 @@ fn render_list(frame: &mut Frame<'_>, area: ratatui::layout::Rect, state: &UiSta
.clients
.iter()
.map(|client| {
let visible_issues: Vec<_> = client.presented_issues().collect();
let identity = client
.identity
.map(|key| short_key(&key))
@@ -520,15 +520,15 @@ fn render_list(frame: &mut Frame<'_>, area: ratatui::layout::Rect, state: &UiSta
(None, None) => "State unavailable".to_owned(),
},
);
let anomaly_marker = if client.issues.is_empty() {
let anomaly_marker = if visible_issues.is_empty() {
String::new()
} else {
" ! anomaly".to_owned()
};
let anomaly_details = if client.issues.is_empty() {
let anomaly_details = if visible_issues.is_empty() {
String::new()
} else {
format!(" {}", safe_text(&format!("{:?}", client.issues)))
format!(" {}", safe_text(&format!("{:?}", visible_issues)))
};
ListItem::new(Line::from(vec![
Span::raw(format!("{row_state}{anomaly_marker} ")),
@@ -652,6 +652,19 @@ fn detail_lines(client: &ClientDisplay, view: &SessionView) -> Vec<Line<'static>
}
)),
];
let has_capability_limitation = client
.issues
.iter()
.any(|issue| client.is_capability_limitation_issue(issue));
if client.has_subnet_routes {
lines.push(Line::from(
"This peer contains subnet routes. Managing this configuration is not supported yet.",
));
} else if has_capability_limitation {
lines.push(Line::from(
"This peer uses valid address routes that this client model cannot manage yet.",
));
}
if let Some(identity) = client.identity {
if let Some(row) = view.client_states.get(&identity) {
lines.push(Line::from(format!(
@@ -697,7 +710,7 @@ fn detail_lines(client: &ClientDisplay, view: &SessionView) -> Vec<Line<'static>
}
}
lines.push(Line::from(format!("Applied state: {:?}", view.applied)));
for issue in &client.issues {
for issue in client.presented_issues() {
lines.push(Line::from(format!(
"Peer issue: {}",
safe_text(&format!("{issue:?}"))
@@ -833,6 +846,44 @@ mod tests {
}
}
fn saved_client_view(identity: PublicKey) -> SessionView {
let client = crate::domain::Client::new(
identity,
"synthetic client",
crate::domain::ClientAddresses {
ipv4: Ipv4Addr::new(10, 0, 0, 2),
ipv6: None,
},
)
.unwrap();
SessionView {
generation: 0,
clients: vec![ClientDisplay {
peer_index: 0,
line: 1,
identity: Some(identity),
label: "synthetic client".to_owned(),
allowed_ips: vec!["10.0.0.2/32".to_owned()],
has_subnet_routes: false,
issues: vec![],
mutable_identity: Some(identity),
}],
client_states: BTreeMap::from([(
identity,
super::super::state::ClientStateRow {
saved: Some(client.clone()),
draft: Some(client),
},
)]),
document_issues: Vec::new(),
applied: super::super::state::AppliedState::Unknown,
reserved_addresses: Default::default(),
statistics: RuntimeObservation::unknown(),
has_draft: false,
vault_initialized: true,
}
}
#[test]
fn bottom_keyboard_panel_has_visible_interior_and_styled_key_labels() {
let state = UiState {
@@ -869,6 +920,9 @@ mod tests {
let style = buffer[key].style();
assert!(style.add_modifier.contains(Modifier::BOLD));
assert_eq!(style.fg, Some(Color::Green));
let action = buffer[(key.0 + 1, key.1)].style();
assert!(matches!(action.fg, None | Some(Color::Reset)));
assert!(!action.add_modifier.contains(Modifier::BOLD));
}
#[test]
@@ -889,6 +943,111 @@ mod tests {
assert!(text.lines().last().unwrap().contains('└'));
}
#[test]
fn requested_terminal_sizes_keep_borders_nonoverlapping_and_footer_inside_panel() {
let state = UiState {
view: Some(empty_view()),
..UiState::default()
};
for (width, height) in [(120, 30), (80, 24), (40, 8), (34, 8)] {
let mut terminal = Terminal::new(TestBackend::new(width, height)).unwrap();
terminal
.draw(|frame| draw(frame, &state, &FormController::default()))
.unwrap();
let buffer = terminal.backend().buffer();
let text = rendered_text(&terminal);
assert!(text.contains("Keys"), "missing footer at {width}x{height}");
let bottom = height - 1;
assert!(buffer[(0, bottom)].symbol().contains('└'));
assert!(buffer[(width - 1, bottom)].symbol().contains('┘'));
assert!(buffer[(1, bottom)].symbol().contains('─'));
assert!(buffer[(0, bottom - 1)].symbol().contains('│'));
assert!(
text.lines()
.any(|line| line.contains("select") || line.contains("new")),
"footer has no visible interior hint at {width}x{height}: {text}"
);
}
}
#[test]
fn list_at_80x24_shows_each_applicable_saved_client_action() {
let identity = PublicKey::from_bytes([4; 32]);
let state = UiState {
selected_identity: Some(identity),
view: Some(saved_client_view(identity)),
..UiState::default()
};
let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap();
terminal
.draw(|frame| draw(frame, &state, &FormController::default()))
.unwrap();
let text = rendered_text(&terminal);
for expected in [
"↑/↓ select",
"Enter details",
"? Help",
"q exit",
"n new",
"r rename",
"d delete",
"p preview",
"c Commit",
"x discard",
"R restart",
"o first file",
"O password reissue",
"b backups / restore",
"B backup",
"i import",
"! issues",
] {
assert!(
text.contains(expected),
"missing {expected:?} in 80x24 render:\n{text}"
);
}
assert!(text.contains("R restart") && text.contains("r rename"));
}
#[test]
fn qr_disclosure_keeps_esc_clear_hint_and_never_renders_fixture_payload() {
let mut forms = FormController::default();
forms.open_disclosure(
crate::delivery::qr::synthetic_test_output(),
"Activation pending".to_owned(),
);
let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap();
terminal
.draw(|frame| draw(frame, &UiState::default(), &forms))
.unwrap();
let text = rendered_text(&terminal);
assert!(text.contains("Secret disclosure"));
assert!(text.contains("Esc clears QR") || text.contains("Esc clear QR"));
assert!(text.contains("clear QR"));
assert!(!text.contains("synthetic QR disclosure fixture"));
let mut compact = Terminal::new(TestBackend::new(34, 8)).unwrap();
compact
.draw(|frame| draw(frame, &UiState::default(), &forms))
.unwrap();
let compact_text = rendered_text(&compact);
assert!(compact_text.contains("clear QR"));
assert!(!compact_text.contains("synthetic QR disclosure fixture"));
}
#[test]
fn below_minimum_terminal_shows_resize_message_without_drawing_footer() {
let mut terminal = Terminal::new(TestBackend::new(33, 7)).unwrap();
terminal
.draw(|frame| draw(frame, &UiState::default(), &FormController::default()))
.unwrap();
let text = rendered_text(&terminal);
assert!(text.contains("Terminal too small"));
assert!(text.contains("34 × 8"));
assert!(!text.contains("Keys"));
}
#[test]
fn compact_form_hints_follow_the_active_form_without_leaking_input() {
let state = UiState::default();
@@ -945,6 +1104,7 @@ mod tests {
identity: Some(identity),
label: "client".to_owned(),
allowed_ips: vec![],
has_subnet_routes: false,
issues: vec![],
mutable_identity: Some(identity),
}],
@@ -1040,6 +1200,7 @@ mod tests {
identity: Some(identity),
label: "new alias".to_owned(),
allowed_ips: vec!["10.20.0.2/32".to_owned()],
has_subnet_routes: false,
issues: vec![],
mutable_identity: Some(identity),
},
@@ -1049,10 +1210,14 @@ mod tests {
identity: Some(unsupported_key),
label: "odd\u{1b}[31m peer".to_owned(),
allowed_ips: vec!["bad\nroute".to_owned()],
issues: vec![crate::wireguard::PeerIssue::UnsupportedAllowedIpShape {
field: 0,
item: 1,
}],
has_subnet_routes: true,
issues: vec![
crate::wireguard::PeerIssue::UnsupportedAllowedIpShape {
field: 0,
item: 1,
},
crate::wireguard::PeerIssue::MalformedAllowedIp { field: 0, item: 2 },
],
mutable_identity: None,
},
],
@@ -1079,7 +1244,7 @@ mod tests {
has_draft: true,
vault_initialized: true,
};
let state = UiState {
let mut state = UiState {
view: Some(view),
..UiState::default()
};
@@ -1100,9 +1265,54 @@ mod tests {
assert!(rendered.contains("inspection only"));
assert!(rendered.contains("removed alias"));
assert!(rendered.contains("Draft removed"));
assert!(rendered.contains("UnsupportedAllowedIpShape"));
assert!(rendered.contains("MalformedAllowedIp"));
assert!(!rendered.contains("UnsupportedAllowedIpShape"));
assert!(
rendered.contains("! anomaly"),
"mixed genuine error stays visible"
);
let subnet_only = ClientDisplay {
has_subnet_routes: true,
issues: vec![crate::wireguard::PeerIssue::UnsupportedAllowedIpShape {
field: 0,
item: 1,
}],
..state.view.as_ref().unwrap().clients[1].clone()
};
assert!(
subnet_only.issues.len() == 1,
"raw safety issue is retained"
);
assert!(subnet_only.presented_issues().next().is_none());
assert!(detail_lines(&state.view.as_ref().unwrap().clients[1], state.view.as_ref().unwrap())
.iter()
.any(|line| line.to_string().contains(
"This peer contains subnet routes. Managing this configuration is not supported yet."
)));
let additional_host = ClientDisplay {
has_subnet_routes: false,
issues: vec![crate::wireguard::PeerIssue::UnsupportedAllowedIpShape {
field: 0,
item: 1,
}],
..state.view.as_ref().unwrap().clients[1].clone()
};
let additional_host_details = detail_lines(&additional_host, state.view.as_ref().unwrap());
assert!(
additional_host_details
.iter()
.any(|line| line.to_string().contains(
"This peer uses valid address routes that this client model cannot manage yet."
))
);
assert!(!additional_host_details.iter().any(|line| {
line.to_string()
.contains("Peer issue: UnsupportedAllowedIpShape")
}));
assert!(rendered.contains("odd\\u{1b}[31m peer"));
assert!(rendered.contains("bad\\nroute"));
state.view.as_mut().unwrap().clients[1].issues =
vec![crate::wireguard::PeerIssue::UnsupportedAllowedIpShape { field: 0, item: 1 }];
let mut narrow = Terminal::new(TestBackend::new(34, 12)).unwrap();
narrow
@@ -1118,7 +1328,8 @@ mod tests {
.collect::<Vec<_>>()
.join("\n");
assert!(narrow_rendered.contains("Draft changed"));
assert!(narrow_rendered.contains("! anomaly"));
assert!(!narrow_rendered.contains("! anomaly"));
assert!(!narrow_rendered.contains("UnsupportedAllowedIpShape"));
assert!(narrow_rendered.contains("Draft removed"));
}
}
+1
View File
@@ -315,6 +315,7 @@ mod tests {
identity: Some(identity),
label: label.to_owned(),
allowed_ips: vec!["10.0.0.2/32".to_owned()],
has_subnet_routes: false,
issues: if mutable {
Vec::new()
} else {
+55 -17
View File
@@ -19,7 +19,7 @@ use super::events::{
};
use super::forms::{FormAction, FormController};
use super::render;
use super::state::{Screen, UiState};
use super::state::{Screen, SessionView, UiState};
use crate::app::Application;
const MIN_WIDTH: u16 = 34;
@@ -765,22 +765,7 @@ fn handle_key(
state.screen = Screen::Message;
return;
};
let mut issues = Vec::new();
issues.extend(
view.document_issues
.iter()
.map(|issue| format!("Document issue: {issue:?}")),
);
for client in &view.clients {
for issue in &client.issues {
issues.push(format!("Peer at line {}: {issue:?}", client.line));
}
}
state.status_message = Some(if issues.is_empty() {
"No document or peer issues were reported.".to_owned()
} else {
issues.join("\n")
});
state.status_message = Some(issue_summary(view));
state.screen = Screen::Message;
state.scroll_offset = 0;
}
@@ -802,6 +787,25 @@ fn handle_key(
}
}
fn issue_summary(view: &SessionView) -> String {
let mut issues = Vec::new();
issues.extend(
view.document_issues
.iter()
.map(|issue| format!("Document issue: {issue:?}")),
);
for client in &view.clients {
for issue in client.presented_issues() {
issues.push(format!("Peer at line {}: {issue:?}", client.line));
}
}
if issues.is_empty() {
"No document or peer issues were reported.".to_owned()
} else {
issues.join("\n")
}
}
/// Handle list/detail navigation and return whether the key was consumed.
/// Keeping this path independent of worker actions makes its state transitions
/// directly testable without starting an application worker.
@@ -921,6 +925,7 @@ mod tests {
identity: None,
label: "unsupported peer".to_owned(),
allowed_ips: Vec::new(),
has_subnet_routes: false,
issues: vec![PeerIssue::MissingIpv4HostRoute],
mutable_identity: None,
};
@@ -933,6 +938,7 @@ mod tests {
identity: Some(first),
label: "first".to_owned(),
allowed_ips: vec!["10.0.0.2/32".to_owned()],
has_subnet_routes: false,
issues: Vec::new(),
mutable_identity: Some(first),
},
@@ -1007,6 +1013,38 @@ mod tests {
assert!(!state.selected_client_mutable());
}
#[test]
fn issues_screen_hides_valid_address_shape_limits_but_keeps_genuine_errors() {
let mut view = navigation_view();
view.clients.truncate(1);
view.clients[0].issues = vec![PeerIssue::UnsupportedAllowedIpShape { field: 0, item: 1 }];
view.clients[0].has_subnet_routes = false;
let additional_host_only = issue_summary(&view);
assert_eq!(
additional_host_only,
"No document or peer issues were reported."
);
assert_eq!(
view.clients[0].issues,
vec![PeerIssue::UnsupportedAllowedIpShape { field: 0, item: 1 }],
"presentation must retain the parser issue for safety checks"
);
view.clients[0].issues = vec![PeerIssue::MissingIpv4HostRoute];
assert_eq!(
issue_summary(&view),
"No document or peer issues were reported.",
"valid IPv6-only address shape is a capability limit"
);
view.clients[0]
.issues
.push(PeerIssue::MalformedAllowedIp { field: 0, item: 2 });
let mixed = issue_summary(&view);
assert!(mixed.contains("MalformedAllowedIp"));
assert!(!mixed.contains("UnsupportedAllowedIpShape"));
}
#[test]
fn restart_results_render_cancel_success_and_uncertain_failure_honestly() {
use crate::app::{RestartFailureKind, RestartReport, RestartStage};
+10
View File
@@ -113,6 +113,10 @@ pub struct PeerInspection {
pub public_key_fields: Vec<DirectiveSpan>,
/// Parsed route values. Unparseable items are represented by `PeerIssue` entries.
pub allowed_ips: Vec<IpNet>,
/// True when a valid parsed route is a subnet rather than a host route.
/// This is a capability limitation for the current client model, not a
/// malformed configuration issue.
pub has_subnet_routes: bool,
pub allowed_ips_fields: Vec<DirectiveSpan>,
/// Parsed alias and safe owned block span, or an explicit metadata issue.
pub metadata: MetadataAttachment,
@@ -346,11 +350,16 @@ impl ConfigDocument {
let mut allowed_ips_fields = Vec::new();
let mut has_ipv4_host_route = false;
let mut has_ipv6_host_route = false;
let mut has_subnet_routes = false;
for (field, (directive, items)) in allowed_fields.into_iter().enumerate() {
allowed_ips_fields.push(directive);
for (item, result) in items {
match result {
Ok(net) => {
has_subnet_routes |= match net {
IpNet::V4(network) => network.prefix_len() < 32,
IpNet::V6(network) => network.prefix_len() < 128,
};
let supported = match net {
IpNet::V4(network) if network.prefix_len() == 32 => {
let first = !has_ipv4_host_route;
@@ -385,6 +394,7 @@ impl ConfigDocument {
public_key_candidates: parsed_keys.into_iter().flatten().collect(),
public_key_fields,
allowed_ips,
has_subnet_routes,
allowed_ips_fields,
metadata: MetadataAttachment::Absent,
issues: peer_issues,