From e4fe4cfced6c81eb13cc186b324c2e3c55acdffa Mon Sep 17 00:00:00 2001 From: Anso Date: Fri, 22 May 2026 13:39:12 -0400 Subject: [PATCH] fix(mesh): address Codex audit findings on F-1 PR (#1158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - docs(sencho-mesh): split subnet_overlap troubleshooting into env-set vs env-unset paths; rewrite the "Customising the mesh subnet" intro to describe the candidate list and the adopt-existing behavior. - backend(MeshService): preserve idempotent 409 handling in the explicit-env path. On createNetwork 409 (TOCTOU race against another process), re-inspect and treat the race-winner as success when its subnet matches the operator's request; subnet_mismatch otherwise. - frontend(MeshDataPlaneBanner): trim the card variant to a true one-line strip (headline only, truncate min-w-0). Full recovery hint stays on the Routing tab variant and in docs. - tests(mesh): add five cases covering the previously untested branches — candidate-loop non-overlap bail, adopt-existing with unparseable subnet, explicit-env generic createNetwork failure, TOCTOU 409 race-winner match, TOCTOU 409 race-winner mismatch. Architecture map (gitignored per Directive 11) updated locally with the new useMeshDataPlane hook node and the mesh.dashboardBanner flow so the local interactive viewer stays accurate. --- .../mesh-setup-error-classification.test.ts | 113 ++++++++++++++++++ backend/src/services/MeshService.ts | 44 +++++-- docs/features/sencho-mesh.mdx | 5 +- .../components/fleet/MeshDataPlaneBanner.tsx | 23 ++-- 4 files changed, 163 insertions(+), 22 deletions(-) diff --git a/backend/src/__tests__/mesh-setup-error-classification.test.ts b/backend/src/__tests__/mesh-setup-error-classification.test.ts index c928f879..bd8c141c 100644 --- a/backend/src/__tests__/mesh-setup-error-classification.test.ts +++ b/backend/src/__tests__/mesh-setup-error-classification.test.ts @@ -327,4 +327,117 @@ describe('MeshService.setupMeshNetwork subnet auto-fallback', () => { expect(status.subnet).toBe('10.42.0.0/24'); expect(createNetwork).toHaveBeenCalledTimes(1); }); + + it('bails the candidate loop on a non-overlap createNetwork failure', async () => { + delete process.env.SENCHO_MESH_SUBNET; + process.env.HOSTNAME = 'sencho'; + const overlap = Object.assign( + new Error('Pool overlaps with other one on this address space'), + { statusCode: 500 }, + ); + const daemonErr = Object.assign( + new Error('daemon attach error: out of inodes'), + { statusCode: 500 }, + ); + // First candidate overlaps (recoverable, advance), second hits a + // generic daemon error (unrecoverable, bail). + const createNetwork = vi.fn() + .mockRejectedValueOnce(overlap) + .mockRejectedValueOnce(daemonErr); + const inspectNetwork = vi.fn().mockRejectedValue({ statusCode: 404, message: 'no such network' }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + const status = svc.getDataPlaneStatus(); + expect(status.ok).toBe(false); + expect(status.reason).toBe('attach_failed'); + // Stops at the second candidate; does not advance to the third. + expect(createNetwork).toHaveBeenCalledTimes(2); + expect(status.subnet).toBe('172.31.0.0/24'); + }); + + it('classifies an unparseable existing subnet as subnet_invalid on the adopt path', async () => { + delete process.env.SENCHO_MESH_SUBNET; + process.env.HOSTNAME = 'sencho'; + const createNetwork = vi.fn(); + const inspectNetwork = vi.fn().mockResolvedValue({ + IPAM: { Config: [{ Subnet: 'not-a-cidr' }] }, + }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + const status = svc.getDataPlaneStatus(); + expect(status.ok).toBe(false); + expect(status.reason).toBe('subnet_invalid'); + expect(status.subnet).toBe('not-a-cidr'); + expect(createNetwork).not.toHaveBeenCalled(); + }); + + it('classifies a generic explicit-env createNetwork failure as attach_failed', async () => { + process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24'; + process.env.HOSTNAME = 'sencho'; + const daemonErr = Object.assign( + new Error('daemon attach error: out of inodes'), + { statusCode: 500 }, + ); + const createNetwork = vi.fn().mockRejectedValue(daemonErr); + const inspectNetwork = vi.fn().mockRejectedValue({ statusCode: 404, message: 'no such network' }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + const status = svc.getDataPlaneStatus(); + expect(status.ok).toBe(false); + expect(status.reason).toBe('attach_failed'); + expect(status.subnet).toBe('10.42.0.0/24'); + }); + + it('treats a TOCTOU 409 in the explicit-env path as idempotent when the race-winner subnet matches', async () => { + process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24'; + process.env.HOSTNAME = 'sencho'; + const conflict = Object.assign( + new Error('network with name sencho_mesh already exists'), + { statusCode: 409 }, + ); + const createNetwork = vi.fn().mockRejectedValue(conflict); + // First inspect (pre-create probe): 404. Second inspect (post-409 + // race-winner check): the matching subnet. + const inspectNetwork = vi.fn() + .mockRejectedValueOnce({ statusCode: 404, message: 'no such network' }) + .mockResolvedValueOnce({ IPAM: { Config: [{ Subnet: '10.42.0.0/24' }] } }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + const status = svc.getDataPlaneStatus(); + expect(status.ok).toBe(true); + expect(status.subnet).toBe('10.42.0.0/24'); + }); + + it('treats a TOCTOU 409 in the explicit-env path as subnet_mismatch when the race-winner differs', async () => { + process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24'; + process.env.HOSTNAME = 'sencho'; + const conflict = Object.assign( + new Error('network with name sencho_mesh already exists'), + { statusCode: 409 }, + ); + const createNetwork = vi.fn().mockRejectedValue(conflict); + const inspectNetwork = vi.fn() + .mockRejectedValueOnce({ statusCode: 404, message: 'no such network' }) + .mockResolvedValueOnce({ IPAM: { Config: [{ Subnet: '172.30.0.0/24' }] } }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + const status = svc.getDataPlaneStatus(); + expect(status.ok).toBe(false); + expect(status.reason).toBe('subnet_mismatch'); + }); }); diff --git a/backend/src/services/MeshService.ts b/backend/src/services/MeshService.ts index 72c74964..48579bdd 100644 --- a/backend/src/services/MeshService.ts +++ b/backend/src/services/MeshService.ts @@ -662,13 +662,43 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { try { await this.createMeshNetwork(envSubnet); } catch (err) { - this.recordSetupFailure( - this.classifyMeshNetworkError(err), - err, - 'error', - envSubnet, - ); - return; + // TOCTOU: another process may have created `sencho_mesh` + // between our inspect (returned null) and our create + // (rejected with 409). Re-inspect; if the existing + // subnet matches what the operator requested, treat + // this as idempotent success (matches the prior + // ensureMeshNetwork 409-then-inspect behavior). Any + // other error or a mismatch reverts to the typed + // failure path. + const dockerErr = err as { statusCode?: number }; + if (dockerErr?.statusCode === 409) { + const raceWinner = await this.inspectExistingMeshSubnet().catch(() => null); + if (raceWinner === envSubnet) { + // Adopt the race-winner's network; proceed to attach. + } else if (raceWinner) { + this.recordSetupFailure( + 'subnet_mismatch', + new Error( + `${SENCHO_MESH_NETWORK} exists with subnet ${raceWinner}, ` + + `expected ${envSubnet}. Remove the network or set SENCHO_MESH_SUBNET to match.`, + ), + 'error', + envSubnet, + ); + return; + } else { + this.recordSetupFailure('attach_failed', err, 'error', envSubnet); + return; + } + } else { + this.recordSetupFailure( + this.classifyMeshNetworkError(err), + err, + 'error', + envSubnet, + ); + return; + } } } } else if (existingSubnet) { diff --git a/docs/features/sencho-mesh.mdx b/docs/features/sencho-mesh.mdx index 736c7db1..7421edfc 100644 --- a/docs/features/sencho-mesh.mdx +++ b/docs/features/sencho-mesh.mdx @@ -154,7 +154,7 @@ Use Test before assuming the issue is your application. It tells you whether the ## Customising the mesh subnet -The default `172.30.0.0/24` will collide if a host already has a Docker bridge in that range. Override it per node with `SENCHO_MESH_SUBNET`: +Sencho picks the mesh subnet automatically. When `SENCHO_MESH_SUBNET` is unset and `sencho_mesh` does not already exist on the Docker daemon, the node walks `172.30.0.0/24`, `172.31.0.0/24`, `10.42.0.0/24`, then `10.43.0.0/24` and keeps the first one Docker accepts. When `sencho_mesh` already exists, Sencho adopts its subnet (Docker is the source of truth across restarts). Override the choice per node with `SENCHO_MESH_SUBNET` when you need a specific CIDR: ```yaml services: @@ -208,7 +208,8 @@ These are the explicit boundaries of the v1 mesh. The fix depends on which reason fired: - - `subnet_overlap`: every candidate subnet overlaps an existing Docker bridge network on this host. Run `docker network ls -q | xargs -L1 docker network inspect --format '{{.Name}} {{range .IPAM.Config}}{{.Subnet}} {{end}}'` to list every existing subnet, then set `SENCHO_MESH_SUBNET` to a free `/24` outside the candidate list and recreate the Sencho container. + - `subnet_overlap` (`SENCHO_MESH_SUBNET` unset): every candidate subnet overlaps an existing Docker bridge network on this host. Run `docker network ls -q | xargs -L1 docker network inspect --format '{{.Name}} {{range .IPAM.Config}}{{.Subnet}} {{end}}'` to list every existing subnet, then set `SENCHO_MESH_SUBNET` to a free `/24` outside the candidate list and recreate the Sencho container. + - `subnet_overlap` (`SENCHO_MESH_SUBNET` set): the explicit CIDR overlaps another Docker bridge network on this host. Either remove the conflicting network or change `SENCHO_MESH_SUBNET` to a free `/24`. Sencho does not auto-fall-back when the operator pins a specific subnet. - `subnet_mismatch`: `SENCHO_MESH_SUBNET` is set explicitly, but `sencho_mesh` already exists with a different subnet. Either remove the network (`docker network rm sencho_mesh` after detaching any containers) or change `SENCHO_MESH_SUBNET` to match the existing subnet. Unset the variable to let Sencho adopt whatever is on disk. - `subnet_invalid`: `SENCHO_MESH_SUBNET` is not a valid CIDR. Fix the value (it must look like `10.42.0.0/24`) and recreate the container. - `ip_in_use`: another container is squatting the IP Sencho wants on its mesh subnet. Find the squatting container with `docker network inspect sencho_mesh`, detach or remove it, then restart Sencho. diff --git a/frontend/src/components/fleet/MeshDataPlaneBanner.tsx b/frontend/src/components/fleet/MeshDataPlaneBanner.tsx index 16ebd4c9..5969190f 100644 --- a/frontend/src/components/fleet/MeshDataPlaneBanner.tsx +++ b/frontend/src/components/fleet/MeshDataPlaneBanner.tsx @@ -39,20 +39,17 @@ export function MeshDataPlaneBanner({ const headline = HEADLINES[status.reason](status); if (variant === 'card') { + // Compact single-row strip for inside the Fleet Heartbeat card. + // Headline only — the full recovery hint lives on the Routing tab + // banner and in /docs/features/sencho-mesh.mdx#troubleshooting. return ( -
- -
- Mesh data plane down · - {status.reason} - · - {headline} - Set - SENCHO_MESH_SUBNET - to a free - /24 - and restart Sencho. -
+
+ + Mesh data plane down + · + {status.reason} + · + {headline}
); }