From 64bf6344a3c883cb39671dec43b07a4681b54b36 Mon Sep 17 00:00:00 2001 From: Anso Date: Fri, 22 May 2026 21:08:56 -0400 Subject: [PATCH] fix(mesh): bias Sencho static IP via IPAM IPRange (F-13) (#1162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mesh): reserve Sencho static IP via IPAM auxiliary address (F-13) Sencho pins itself to +2 on sencho_mesh, but the IPAM block only declared Subnet, so Docker freely handed that address to any meshed workload that restarted while Sencho was offline. A real-world hit on arrapps-prod during a Sencho upgrade had tautulli grab 172.30.0.2, which blocked the new Sencho container's mesh attach with "Address already in use" and left compose in a half-state needing manual disconnect/recreate. The fix reserves +2 via AuxiliaryAddresses on the IPAM Config when creating sencho_mesh. Aux-listed addresses are removed from the auto-allocatable pool, so Docker refuses to hand the IP to any container that does not explicitly request it. Sencho's own attach via connectContainerToNetwork({ ipv4Address }) is unaffected because explicit pins still bind aux-reserved addresses. Workload containers without a pinned IP get .3 and up. Wire format verified against the Docker Engine REST v1.33 OpenAPI spec: the JSON key on POST /networks/create (and on the inspect response) is AuxiliaryAddresses inside each IPAM.Config item, value { sencho: }. Adopt-existing path: when Sencho boots against a sencho_mesh that pre-dates this reservation, the data plane still comes up but a one-time warn fires (mesh.enable activity at level: 'warn' plus a [Mesh] console line so docker logs surfaces it). The advisory explains the squat risk and gives the recreate recipe. Tests: 5 cases added to mesh-setup-error-classification.test.ts covering the explicit-env create payload, the candidate-iteration winner payload, the adopt-legacy warn (env-unset), the adopt-already-reserved silent path, and the TOCTOU 409 race-winner adopt-legacy warn. Docs: one sentence added to docs/features/sencho-mesh.mdx under "Customising the mesh subnet" describing the reservation positively. No tier/role/capability/flag gates touched (no frontend changes). * fix(mesh): use IPRange upper-half bias instead of aux-address reservation The initial F-13 fix used IPAM AuxiliaryAddresses to reserve +2 on sencho_mesh. Empirical probe against Docker 29.4.3 confirmed this also blocks explicit pins via EndpointConfig.IPAMConfig.IPv4Address: libnetwork's RequestAddress() rejects a preferred-address request when the bit is already set by the aux reservation. Result: the freshly- reserved network refuses Sencho's own ensureSelfAttached, and the data plane never comes up. The pivot uses IPRange instead. IPRange constrains Docker's auto- allocation to the configured CIDR; preferred-address requests via RequestAddress(prefAddress) skip the IPRange check and only consult the subnet-wide bitmap. So setting IPRange to the upper half of the subnet (e.g. 172.30.0.128/25 for 172.30.0.0/24) biases workloads without an explicit IP to +128 and up, while Sencho's explicit pin to +2 still succeeds. Verified on Docker 29.4.3 with the same workstation that produced the original audit: - `docker run --rm --network N --ip 10.99.99.2` against a network with `--aux-address sencho=10.99.99.2` → "Address already in use" (rejects explicit pin). - Same `--ip` against a network with `--ip-range 10.99.99.128/25` → succeeds (10.99.99.2 is outside the range but inside the subnet). - Auto-allocated workload on the IPRange network lands at .129. Adopt-existing legacy detection now compares IPRange instead of AuxiliaryAddresses. inspectExistingMeshSubnet returns { subnet, ipRange } and the warn fires when ipRange differs from the expected upper-half CIDR. Same once-per-process semantics as before. createMeshNetwork now derives the IPRange via a new getMeshIpRangeFromSubnet helper. The five regression tests assert IPRange = +128/ in the create payload and the expected/actual IPRange in the legacy-warn details. --- .../mesh-setup-error-classification.test.ts | 151 +++++++++++++++++ backend/src/services/DockerController.ts | 8 +- backend/src/services/MeshService.ts | 154 ++++++++++++++++-- docs/features/sencho-mesh.mdx | 2 +- 4 files changed, 297 insertions(+), 18 deletions(-) diff --git a/backend/src/__tests__/mesh-setup-error-classification.test.ts b/backend/src/__tests__/mesh-setup-error-classification.test.ts index e907af8a..563bb7ef 100644 --- a/backend/src/__tests__/mesh-setup-error-classification.test.ts +++ b/backend/src/__tests__/mesh-setup-error-classification.test.ts @@ -442,6 +442,157 @@ describe('MeshService.setupMeshNetwork subnet auto-fallback', () => { }); }); +describe('MeshService static IP reservation (F-13)', () => { + it('emits IPRange for the upper half of the subnet when creating with an explicit env subnet', async () => { + process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24'; + process.env.HOSTNAME = 'sencho'; + const createNetwork = vi.fn().mockResolvedValue(undefined); + const inspectNetwork = vi.fn().mockRejectedValue({ statusCode: 404, message: 'no such network' }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + expect(svc.getDataPlaneStatus().ok).toBe(true); + expect(createNetwork).toHaveBeenCalledTimes(1); + const payload = createNetwork.mock.calls[0][0] as { + IPAM?: { Config?: Array<{ Subnet?: string; IPRange?: string }> }; + }; + expect(payload.IPAM?.Config?.[0]?.Subnet).toBe('10.42.0.0/24'); + expect(payload.IPAM?.Config?.[0]?.IPRange).toBe('10.42.0.128/25'); + }); + + it('emits the IPRange on the winning candidate when env is unset and the first candidate overlaps', 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 createNetwork = vi.fn() + .mockRejectedValueOnce(overlap) + .mockResolvedValueOnce(undefined); + const inspectNetwork = vi.fn().mockRejectedValue({ statusCode: 404, message: 'no such network' }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + expect(svc.getDataPlaneStatus().ok).toBe(true); + expect(svc.getDataPlaneStatus().subnet).toBe('172.31.0.0/24'); + expect(createNetwork).toHaveBeenCalledTimes(2); + const winningPayload = createNetwork.mock.calls[1][0] as { + IPAM?: { Config?: Array<{ Subnet?: string; IPRange?: string }> }; + }; + expect(winningPayload.IPAM?.Config?.[0]?.Subnet).toBe('172.31.0.0/24'); + expect(winningPayload.IPAM?.Config?.[0]?.IPRange).toBe('172.31.0.128/25'); + }); + + it('emits a legacy warn when adopting a sencho_mesh without the upper-half IPRange', async () => { + delete process.env.SENCHO_MESH_SUBNET; + process.env.HOSTNAME = 'sencho'; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const createNetwork = vi.fn(); + // Legacy network: Subnet only, no IPRange (Docker defaults auto- + // allocation to the entire subnet, including .2). + const inspectNetwork = vi.fn().mockResolvedValue({ + 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(true); + expect(status.subnet).toBe('172.30.0.0/24'); + expect(createNetwork).not.toHaveBeenCalled(); + + const all = (svc as unknown as { activity: MeshActivityEvent[] }).activity; + const warns = all.filter((e) => e.level === 'warn' && /without the upper-half IPAM IPRange/.test(e.message)); + expect(warns).toHaveLength(1); + expect(warns[0].details).toMatchObject({ + subnet: '172.30.0.0/24', + expectedIpRange: '172.30.0.128/25', + actualIpRange: null, + }); + + const meshLines = warnSpy.mock.calls + .map((args) => String(args[0])) + .filter((line) => /\[Mesh\] sencho_mesh adopted without the upper-half IPAM IPRange/.test(line)); + expect(meshLines.length).toBeGreaterThan(0); + }); + + it('stays silent on the adopt path when the existing sencho_mesh already carries the upper-half IPRange', async () => { + delete process.env.SENCHO_MESH_SUBNET; + process.env.HOSTNAME = 'sencho'; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const createNetwork = vi.fn(); + const inspectNetwork = vi.fn().mockResolvedValue({ + IPAM: { + Config: [{ + Subnet: '172.30.0.0/24', + IPRange: '172.30.0.128/25', + }], + }, + }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + const status = svc.getDataPlaneStatus(); + expect(status.ok).toBe(true); + + const all = (svc as unknown as { activity: MeshActivityEvent[] }).activity; + const warns = all.filter((e) => e.level === 'warn' && /IPAM IPRange/.test(e.message)); + expect(warns).toHaveLength(0); + + const meshLines = warnSpy.mock.calls + .map((args) => String(args[0])) + .filter((line) => /IPAM IPRange/.test(line)); + expect(meshLines).toHaveLength(0); + }); + + it('emits the legacy warn when a TOCTOU 409 race-winner is a legacy network', async () => { + // Operator-explicit subnet matches a race-winner that pre-dates the + // upper-half IPRange bias: another process created sencho_mesh + // between our initial inspect and our create. We adopt it, the data + // plane comes up, and the warn fires because the race-winner's IPAM + // block has no IPRange entry. Exercises the subtle assignment in + // setupMeshNetwork that reassigns existingSubnet = raceWinner. + process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24'; + process.env.HOSTNAME = 'sencho'; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + 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: '10.42.0.0/24' }] } }); + mockDocker({ createNetwork, inspectNetwork }); + + const svc = MeshService.getInstance(); + await callSetup(svc); + + expect(svc.getDataPlaneStatus().ok).toBe(true); + const all = (svc as unknown as { activity: MeshActivityEvent[] }).activity; + const warns = all.filter((e) => e.level === 'warn' && /without the upper-half IPAM IPRange/.test(e.message)); + expect(warns).toHaveLength(1); + expect(warns[0].details).toMatchObject({ + subnet: '10.42.0.0/24', + expectedIpRange: '10.42.0.128/25', + actualIpRange: null, + }); + const meshLines = warnSpy.mock.calls + .map((args) => String(args[0])) + .filter((line) => /\[Mesh\] sencho_mesh adopted without the upper-half IPAM IPRange/.test(line)); + expect(meshLines.length).toBeGreaterThan(0); + }); +}); + describe('MeshService console.log mirror (F-5)', () => { it('mirrors a setup failure to console.error with the [Mesh] prefix', async () => { process.env.SENCHO_MESH_SUBNET = '10.42.0.0/24'; diff --git a/backend/src/services/DockerController.ts b/backend/src/services/DockerController.ts index c7233be1..825ee3c4 100644 --- a/backend/src/services/DockerController.ts +++ b/backend/src/services/DockerController.ts @@ -96,7 +96,13 @@ export type NetworkDriver = 'bridge' | 'overlay' | 'macvlan' | 'host' | 'none'; export interface CreateNetworkOptions { Name: string; Driver?: NetworkDriver; - IPAM?: { Config: Array<{ Subnet?: string; Gateway?: string }> }; + IPAM?: { + Config: Array<{ + Subnet?: string; + Gateway?: string; + IPRange?: string; + }>; + }; Labels?: Record; Internal?: boolean; Attachable?: boolean; diff --git a/backend/src/services/MeshService.ts b/backend/src/services/MeshService.ts index e1010174..77827d5f 100644 --- a/backend/src/services/MeshService.ts +++ b/backend/src/services/MeshService.ts @@ -79,6 +79,42 @@ export function getSenchoIpFromSubnet(subnet: string): string { ].join('.'); } +/** + * Returns the `IPRange` CIDR that biases Docker's IPAM auto-allocation to + * the upper half of `subnet`, leaving the lower half (which contains + * Sencho's static `+2`) free for explicit pins only. For the + * default `172.30.0.0/24` this is `172.30.0.128/25`. + * + * The IPRange constrains auto-allocation only: workload containers without + * an explicit `ipv4Address` get `+128` and up, never the reserved + * low addresses. Explicit pins inside the Subnet but outside the IPRange + * still succeed (libnetwork's `RequestAddress` checks the bitmap, not the + * IPRange, when the caller supplies a preferred address), so Sencho's own + * `ensureSelfAttached` can still bind `+2`. + * + * Throws on invalid CIDR or a prefix narrower than `/30` (no upper half to + * carve). The candidate list is `/24` so this never trips in practice. + */ +export function getMeshIpRangeFromSubnet(subnet: string): string { + const cidr = subnet.trim().match(/^(\d+)\.(\d+)\.(\d+)\.(\d+)\/(\d+)$/); + if (!cidr) throw new Error(`Invalid mesh subnet CIDR: ${subnet}`); + const octets = [Number(cidr[1]), Number(cidr[2]), Number(cidr[3]), Number(cidr[4])]; + const prefix = Number(cidr[5]); + if (octets.some((o) => o < 0 || o > 255) || prefix < 8 || prefix > 29) { + throw new Error(`Invalid mesh subnet CIDR: ${subnet}`); + } + const ipInt = (octets[0] << 24) | (octets[1] << 16) | (octets[2] << 8) | octets[3]; + const mask = prefix === 0 ? 0 : (0xffffffff << (32 - prefix)) >>> 0; + const network = (ipInt & mask) >>> 0; + const upperHalfStart = (network + (1 << (32 - prefix - 1))) >>> 0; + return [ + (upperHalfStart >>> 24) & 0xff, + (upperHalfStart >>> 16) & 0xff, + (upperHalfStart >>> 8) & 0xff, + upperHalfStart & 0xff, + ].join('.') + '/' + (prefix + 1); +} + /** * Discriminator for why the mesh data plane is or is not healthy. Set by * `setupMeshNetwork` and exposed through `getDataPlaneStatus()` so @@ -646,7 +682,7 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { } } - let existingSubnet: string | null; + let existingSubnet: { subnet: string; ipRange: string | null } | null; try { existingSubnet = await this.inspectExistingMeshSubnet(); } catch (err) { @@ -663,11 +699,11 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { } if (envSubnet) { - if (existingSubnet && existingSubnet !== envSubnet) { + if (existingSubnet && existingSubnet.subnet !== envSubnet) { this.recordSetupFailure( 'subnet_mismatch', new Error( - `${SENCHO_MESH_NETWORK} exists with subnet ${existingSubnet}, ` + + `${SENCHO_MESH_NETWORK} exists with subnet ${existingSubnet.subnet}, ` + `expected ${envSubnet}. Remove the network or set SENCHO_MESH_SUBNET to match.`, ), 'error', @@ -690,13 +726,17 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { const dockerErr = err as { statusCode?: number }; if (dockerErr?.statusCode === 409) { const raceWinner = await this.inspectExistingMeshSubnet().catch(() => null); - if (raceWinner === envSubnet) { + if (raceWinner && raceWinner.subnet === envSubnet) { // Adopt the race-winner's network; proceed to attach. + // Defer the legacy-reservation check to the shared + // helper below so the warn message is identical + // regardless of which branch arrived here. + existingSubnet = raceWinner; } else if (raceWinner) { this.recordSetupFailure( 'subnet_mismatch', new Error( - `${SENCHO_MESH_NETWORK} exists with subnet ${raceWinner}, ` + + `${SENCHO_MESH_NETWORK} exists with subnet ${raceWinner.subnet}, ` + `expected ${envSubnet}. Remove the network or set SENCHO_MESH_SUBNET to match.`, ), 'error', @@ -720,10 +760,10 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { } } else if (existingSubnet) { try { - this.senchoIp = getSenchoIpFromSubnet(existingSubnet); - this.meshSubnet = existingSubnet; + this.senchoIp = getSenchoIpFromSubnet(existingSubnet.subnet); + this.meshSubnet = existingSubnet.subnet; } catch (err) { - this.recordSetupFailure('subnet_invalid', err, 'error', existingSubnet); + this.recordSetupFailure('subnet_invalid', err, 'error', existingSubnet.subnet); return; } } else { @@ -776,6 +816,23 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { } } + // Legacy-network advisory: if we adopted an existing `sencho_mesh` + // (rather than creating a fresh one) AND its IPAM block lacks the + // upper-half IPRange that biases Docker's auto-allocation away from + // `+2`, warn the operator that a meshed workload + // restarting while Sencho is offline can squat that IP (F-13 + // failure mode). The data plane still comes up; the warn is + // advisory and rides at `level: 'warn'` on the existing + // `mesh.enable` activity type plus a console mirror so it appears + // in `docker logs sencho`. Fresh-created networks always carry + // the bias (see `createMeshNetwork`). + if (existingSubnet && this.senchoIp) { + const expectedIpRange = getMeshIpRangeFromSubnet(this.meshSubnet); + if (existingSubnet.ipRange !== expectedIpRange) { + this.warnLegacyMeshReservation(existingSubnet.ipRange); + } + } + try { await this.ensureSelfAttached(); } catch (err) { @@ -800,18 +857,65 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { } /** - * Return the subnet of an existing `sencho_mesh` network, or null if the - * network does not exist. Docker's inspect endpoint surfaces 404 for - * the missing case; any other error is re-raised so the caller can - * classify it as `attach_failed`. + * Emit the F-13 advisory: the operator is running on a `sencho_mesh` + * that pre-dates the upper-half IPRange bias, so Docker's auto- + * allocation can still hand Sencho's `+2` IP to another + * meshed workload while Sencho is offline. Logged once per process at + * boot (this is only called from `setupMeshNetwork`, which itself + * runs once per `start()`). Mirrored to `console.warn` so it appears + * in `docker logs sencho`. */ - private async inspectExistingMeshSubnet(): Promise { + private warnLegacyMeshReservation(actualIpRange: string | null): void { + const expected = this.senchoIp ? getMeshIpRangeFromSubnet(this.meshSubnet) : null; + const msg = `${SENCHO_MESH_NETWORK} adopted without the upper-half IPAM IPRange ` + + `(expected ${expected ?? 'a constrained IPRange'}); another container can squat ${this.senchoIp} ` + + `while Sencho is offline. To apply the bias: stop every meshed stack, ` + + `run \`docker network rm ${SENCHO_MESH_NETWORK}\`, then restart Sencho.`; + this.logActivity({ + source: 'mesh', + level: 'warn', + type: 'mesh.enable', + message: msg, + details: { + subnet: this.meshSubnet, + expectedIpRange: expected, + actualIpRange, + }, + }); + console.warn(`[Mesh] ${msg}`); + } + + /** + * Return the subnet and the IPRange of an existing `sencho_mesh` + * network, or null if the network does not exist. The `ipRange` field + * reflects `IPAM.Config[0].IPRange` on the daemon: null when the + * network pre-dates the upper-half bias or the operator chose not to + * set one, the configured CIDR otherwise. The adopt-existing path + * uses this to surface a one-time warn when running on a legacy + * network that is vulnerable to IP squatting while Sencho is offline. + * Docker's inspect endpoint surfaces 404 for the missing-network + * case; any other error is re-raised so the caller can classify it + * as `attach_failed`. + */ + private async inspectExistingMeshSubnet(): Promise< + { subnet: string; ipRange: string | null } | null + > { const dc = DockerController.getInstance(NodeRegistry.getInstance().getDefaultNodeId()); try { const info = await dc.inspectNetwork(SENCHO_MESH_NETWORK) as { - IPAM?: { Config?: Array<{ Subnet?: string }> }; + IPAM?: { + Config?: Array<{ + Subnet?: string; + IPRange?: string; + }>; + }; } | undefined; - return info?.IPAM?.Config?.[0]?.Subnet ?? null; + const cfg = info?.IPAM?.Config?.[0]; + if (!cfg?.Subnet) return null; + return { + subnet: cfg.Subnet, + ipRange: cfg.IPRange ?? null, + }; } catch (err) { const e = err as { statusCode?: number }; if (e?.statusCode === 404) return null; @@ -824,14 +928,32 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { * the raw Dockerode error (including the 500 pool-overlap that * `classifyMeshNetworkError` recognises) so callers can decide whether * to retry on another candidate or bail. + * + * The IPAM block sets `IPRange` to the upper half of the subnet so + * Docker's auto-allocation pulls from `+128` and up, leaving + * the lower half (which contains Sencho's static `+2`) free + * for explicit pins only. This stops a meshed workload that restarts + * while Sencho is offline from grabbing `.2` by default. Sencho's own + * attach via `connectContainerToNetwork({ ipv4Address })` still binds + * `.2` because explicit pins can land anywhere in the subnet, not + * just inside the IPRange. (Aux-address reservation would also block + * the IP from auto-allocation, but libnetwork's `RequestAddress` + * rejects explicit pins to aux-reserved addresses too, which would + * defeat Sencho's own self-attach — verified against Docker 29.4.3.) */ private async createMeshNetwork(subnet: string): Promise { const dc = DockerController.getInstance(NodeRegistry.getInstance().getDefaultNodeId()); + const ipRange = getMeshIpRangeFromSubnet(subnet); await dc.createNetwork({ Name: SENCHO_MESH_NETWORK, Driver: 'bridge', Attachable: true, - IPAM: { Config: [{ Subnet: subnet }] }, + IPAM: { + Config: [{ + Subnet: subnet, + IPRange: ipRange, + }], + }, Labels: { 'io.sencho.mesh': 'true' }, }); } diff --git a/docs/features/sencho-mesh.mdx b/docs/features/sencho-mesh.mdx index 2c26da0a..b5d697d1 100644 --- a/docs/features/sencho-mesh.mdx +++ b/docs/features/sencho-mesh.mdx @@ -163,7 +163,7 @@ services: - SENCHO_MESH_SUBNET=10.42.0.0/24 ``` -Sencho pins itself to ` + 2` (so `10.42.0.2` for the example above; the bridge's `+1` gateway sits between). Each node is configured independently; the alias registry pushes the correct local IP to each node's override file. +Sencho pins itself to ` + 2` (so `10.42.0.2` for the example above; the bridge's `+1` gateway sits between). The `sencho_mesh` network is created with an IPRange covering the upper half of the subnet, so Docker's auto-allocation hands meshed workloads ` + 128` and up by default and never grabs Sencho's static IP while Sencho is offline. Each node is configured independently; the alias registry pushes the correct local IP to each node's override file. ## Security and trust boundaries