Skip to content

wireguard: warn about an overlapping client subnet while typing - #153

Open
ispyisail wants to merge 1 commit into
masterfrom
wireguard/subnet-overlap-warning
Open

wireguard: warn about an overlapping client subnet while typing#153
ispyisail wants to merge 1 commit into
masterfrom
wireguard/subnet-overlap-warning

Conversation

@ispyisail

Copy link
Copy Markdown
Owner

Adds a live warning when the "subnet behind client" overlaps the router's LAN or the WireGuard server subnet, instead of only refusing once the admin hits save.

Why

validateAc() already rejects an overlapping routed subnet (the forum-18405 lockout backstop). That refusal is correct and stays exactly as it is — but the admin only finds out after filling in the whole form and pressing save. Warning as they type is a cheaper signal for the same hazard.

This mirrors the approach lantis1008/gargoyle took in lantis1008#72, with one deliberate difference noted below.

What changed

  • wgSubnetOverlapErrors() — the overlap detection lifted verbatim out of validateAc() into a shared helper. validateAc() now calls it and pushes whatever it returns, so the save-time behaviour and message set are unchanged, including the case where both the LAN and WG-server messages apply.
  • checkWgClientSubnetOverlap() — shows the matching message in a banner in the client modal, hides it when the subnet is clear.
  • Template: oninput on the two subnet fields, plus the (initially hidden, empty) banner.

No new i18n strings — the banner reuses wgStr.SubOvLan / wgStr.SubOvWg, so the warning is worded identically to the refusal it predicts.

Two things done deliberately

One source of truth for the overlap rule. Rather than adding a second copy of the subnet-overlap maths for the live check, both paths call the same helper. A banner that disagreed with the save-time refusal would be worse than no banner.

The banner is re-evaluated on modal open, not only from oninput. oninput fires only on real typing; the client-load path sets .value programmatically, which does not fire it. Without a re-check, a warning raised for one client stays on screen when the next client's modal opens over it. setAllowedClientVisibility() runs on every modal open and is where that re-check lives. This is the same bug fixed in lantis1008/gargoyle@2753d9e6 after lantis1008#72 merged; building the call in from the start avoids inheriting it.

Testing

  • node --check clean.
  • The extracted helper and the banner were exercised directly (real functions pulled out of wireguard.js, not reimplemented): clean subnet, LAN overlap, WG-server overlap, a supernet containing the LAN, both-overlaps-at-once, banner clears for a clean subnet, banner stays hidden while the section is collapsed, half-typed address does not flash, and a missing banner element is a safe no-op. 10/10.
  • vnet phase 60 (60-wireguard_subnet_lockout.test.sh) covers the save-time refusal this refactor must not change, and is the regression guard for it.

Extract the overlap detection validateAc() already does into a shared
helper, and use it to show a warning as the subnet is entered rather
than only refusing on save. The save-time refusal is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant