From b6efcff25e93ddc44422a4c323f02c88e6faf2a7 Mon Sep 17 00:00:00 2001 From: William Mantly Date: Mon, 10 Aug 2026 20:18:39 -0400 Subject: [PATCH] fix(mesh): peer removal now cleans up its kernel routes wg_iface.removePeer() previously just did `wg set ... remove` -- the kernel routes setPeer() adds for a peer's AllowedIPs (since wg itself only configures crypto-routing, not kernel routes -- see setPeer's own comment) were never cleaned up, a real TODO flagged in code but never exercised because nothing removed a mesh peer at all. - removePeer() now queries the peer's current AllowedIPs (`wg show allowed-ips`) BEFORE removing it -- once gone, wg no longer knows what to clean up -- and issues `ip route del` for each. - New DELETE /api/mesh/gateways/:id (models/mesh_gateway.js gained remove()) actually calls removePeer(), so the fix has a real caller; previously there was no removal path anywhere in the mesh feature at all. Refuses to remove the local "(self)" entry. Does not reach out to the remote gateway to remove the reciprocal peer -- that side needs the same action taken independently. - Mesh UI: remove button per non-self peer row, using app.messages.confirm (not native confirm() -- caught by this repo's own no-native-dialogs test, which failed on first pass and is now green). Verified for real with a live WireGuard interface in a container: routes for a peer's AllowedIPs present after setPeer, confirmed gone after removePeer, while the interface's own local route correctly survives. --- nodejs/models/mesh_gateway.js | 11 ++++++++++- nodejs/routes/mesh.js | 21 +++++++++++++++++++++ nodejs/utils/wg_iface.js | 26 ++++++++++++++++++++++---- nodejs/views/mesh.ejs | 15 ++++++++++++++- 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/nodejs/models/mesh_gateway.js b/nodejs/models/mesh_gateway.js index f301080..8a24eab 100644 --- a/nodejs/models/mesh_gateway.js +++ b/nodejs/models/mesh_gateway.js @@ -82,4 +82,13 @@ async function register({ publicKey, endpoint, siteSlug }) { return gateway; } -module.exports = { list, findByPublicKey, register, MAX_MESH_INDEX }; +async function remove(id) { + const redis = await getRedis(); + const gw = deserialize(await redis.hGetAll(gatewayKey(id))); + if (!gw) return null; + await redis.del(gatewayKey(id)); + await redis.zRem(idxKey(), id); + return gw; +} + +module.exports = { list, findByPublicKey, register, remove, MAX_MESH_INDEX }; diff --git a/nodejs/routes/mesh.js b/nodejs/routes/mesh.js index bed1922..6908c0a 100644 --- a/nodejs/routes/mesh.js +++ b/nodejs/routes/mesh.js @@ -161,4 +161,25 @@ router.get('/gateways', middleware.auth, middleware.requireJumpAdmin, async (req } catch (e) { next(e); } }); +// Remove a peer gateway: tears down its local WG peer entry + kernel routes +// (wgIface.removePeer) and drops it from the registry. Does NOT reach out to +// the remote gateway to remove the reciprocal peer entry there -- an admin +// on that side needs to do the same. Refuses to remove the self-entry +// ("(self)"), since that's this gateway's own identity, not a peer. +router.delete('/gateways/:id', middleware.auth, middleware.requireJumpAdmin, async (req, res, next) => { + try { + const gateways = await meshGateway.list(); + const target = gateways.find((g) => g.id === req.params.id); + if (!target) return res.status(404).json({ status: 'error', message: 'gateway not found' }); + if (target.siteSlug === '(self)') { + return res.status(400).json({ status: 'error', message: 'cannot remove this gateway\'s own self-entry' }); + } + + wgIface.removePeer(IFACE, target.publicKey); + await meshGateway.remove(target.id); + + res.json({ status: 'ok', removed: { id: target.id, siteSlug: target.siteSlug, meshIndex: target.meshIndex } }); + } catch (e) { next(e); } +}); + module.exports = router; diff --git a/nodejs/utils/wg_iface.js b/nodejs/utils/wg_iface.js index 6812b64..a368a25 100644 --- a/nodejs/utils/wg_iface.js +++ b/nodejs/utils/wg_iface.js @@ -137,12 +137,30 @@ function setPeer(name, { publicKey, endpoint, allowedIPs, keepalive }) { } } -// TODO: doesn't clean up the kernel routes setPeer added for this peer's -// AllowedIPs (would need to record or query them first) -- not exercised by -// any caller yet (nothing in this codebase removes a mesh peer today), but -// flagging so whoever adds that doesn't get bitten by stale routes. +// Removes the peer AND the kernel routes setPeer() added for its +// AllowedIPs -- query them BEFORE removing the peer (once gone, `wg` no +// longer knows what to clean up, and nothing else tracks these routes, +// since they were added by us directly, not by wg-quick). +// +// Safe to assume none of a peer's AllowedIPs collide with this gateway's +// own local address range: mesh indexes are unique per gateway +// (models/mesh_gateway.js's nextFreeMeshIndex), so a peer's +// 172.24..0/24 can never equal our own 172.24..0/24. function removePeer(name, publicKey) { + const show = tryRun('wg', ['show', name, 'allowed-ips']); + let allowedIPs = []; + if (show.ok) { + const line = show.out.split('\n').find((l) => l.startsWith(publicKey + '\t')); + if (line) { + allowedIPs = (line.split('\t')[1] || '').split(/\s+/).filter((ip) => ip && ip !== '(none)'); + } + } + tryRun('wg', ['set', name, 'peer', publicKey, 'remove']); + + for (const cidr of allowedIPs) { + tryRun('ip', ['route', 'del', cidr, 'dev', name]); + } } module.exports = { diff --git a/nodejs/views/mesh.ejs b/nodejs/views/mesh.ejs index a4bcfc2..97f0b44 100644 --- a/nodejs/views/mesh.ejs +++ b/nodejs/views/mesh.ejs @@ -109,7 +109,7 @@ function renderGatewaysTable(gateways) { return; } var html = '' - + '' + + '' + ''; gateways.forEach(function(g){ var isSelf = g.siteSlug === '(self)'; @@ -120,12 +120,25 @@ function renderGatewaysTable(gateways) { + '' + '' + '' + + '' + ''; }); html += '
SiteMesh IndexMesh AddressEndpointPublic KeyLast SeenSiteMesh IndexMesh AddressEndpointPublic KeyLast SeenActions
' + esc(g.endpoint || '—') + '' + esc(g.publicKey) + '' + (g.lastSeenAt ? new Date(Number(g.lastSeenAt)).toLocaleString() : '—') + '' + (isSelf ? '' : + '') + + '
'; $w.html(html); } +async function removeMeshGateway(id, label) { + var ok = await app.messages.confirm('Remove mesh peer "' + label + '"? This tears down the local WireGuard peer + routes. The other side keeps its half until removed there too.', $('#mesh-gateways-wrap'), 'danger'); + if (!ok) return; + app.api.delete('mesh/gateways/' + id, function(err){ + if (err) return app.messages.toast('Failed to remove gateway: ' + err.message, 'danger'); + app.messages.toast('Gateway removed', 'success'); + loadMeshStatus(); + }); +} + function mintMeshJoinToken() { app.api.post('mesh/join-tokens', {}, function(err, data){ if (err) return app.messages.toast('Failed to mint join token: ' + err.message, 'danger');