From 02a09688bcefe55fe86bb33ca0df9ee565c85bf1 Mon Sep 17 00:00:00 2001 From: Anso Date: Sat, 24 Oct 2026 17:10:20 -0400 Subject: [PATCH] fix(mesh): trust only readable compose declarations when releasing ports The declaration read is the only evidence that frees a saved port claim, so content that is not Compose cannot be treated as an authored removal. An explicitly supplied `services` value must now be a service mapping, and a service definition must be a mapping too. A sequence saved by an editor used to contribute its array indexes as service names, which dropped a real service from the snapshot and freed its port. A non-mapping document now reports unknown instead of contributing names. An auxiliary layer with no `services` key contributes no names rather than voiding the union. A networks-only or volumes-only override is valid Compose, and an authored removal beside it releases the claim as documented. A discovered file chain selected by COMPOSE_FILE is unknown, matching the effective-anatomy rule: this reader does not follow that chain, so the files it would read are not the project's files. An explicit Git file list ignores COMPOSE_FILE and is still read, because that list is what Compose loads. Tests cover each invalid shape at the refresh and route boundaries, the environment-selected chain, both auxiliary layer shapes, the preserved Git list, and the malformed-layer case. --- .../__tests__/mesh-alias-stability.test.ts | 193 ++++++++++++++++++ .../mesh-local-services-route.test.ts | 16 ++ backend/src/services/MeshService.ts | 53 ++++- docs/features/sencho-mesh.mdx | 2 +- 4 files changed, 254 insertions(+), 10 deletions(-) diff --git a/backend/src/__tests__/mesh-alias-stability.test.ts b/backend/src/__tests__/mesh-alias-stability.test.ts index 6c93e2cba..7c45d733c 100644 --- a/backend/src/__tests__/mesh-alias-stability.test.ts +++ b/backend/src/__tests__/mesh-alias-stability.test.ts @@ -617,6 +617,199 @@ describe('partial container removal keeps still-declared claims', () => { }); }); +/** + * The declaration read is the only evidence that releases a saved claim, so + * it has to be trustworthy. A saved compose file can hold anything an editor + * wrote, and a discovered chain is not the chain Compose loads when + * COMPOSE_FILE selects its own. None of those states says a service is gone. + */ +describe('declaration reconciliation trusts only readable compose content', () => { + function seedClaim( + stack: string, + services: Array<{ service: string; ports: number[] }>, + ): { id: number; name: string } { + const db = DatabaseService.getInstance(); + const localNode = db.getDefaultNode(); + if (!localNode) throw new Error('missing default node'); + db.insertMeshStack(localNode.id, stack, 'tester'); + seedLastKnown(localNode.id, stack, services); + return localNode; + } + + function savedServices(nodeId: number, stack: string): Array<{ service: string; ports: number[] }> { + const row = DatabaseService.getInstance().listMeshStacks(nodeId).find((r) => r.stack_name === stack); + return parseLastKnownServices(row?.last_known_services); + } + + /** + * A declaration saved as an array or a scalar is not Compose content, so + * it says nothing about which services the stack used to own. The saved + * claim stays, and a competing opt-in on its port is still refused. + */ + async function expectInvalidDeclarationKeepsClaim(stack: string, contents: string): Promise { + const localNode = seedClaim(stack, [{ service: 'db', ports: [5432] }]); + const host = `db.${stack}.${localNode.name}.sencho`; + writeRawComposeFile(stack, contents); + + await withLocalDocker(async () => []); + const svc = MeshService.getInstance(); + await svc.refreshAliasCache(); + + expect(savedServices(localNode.id, stack)).toEqual([{ service: 'db', ports: [5432] }]); + expect((svc as unknown as Internals).reservedPorts.get(5432)?.host).toBe(host); + + await withLocalDocker(async (opts) => ( + stackFromFilter(opts) === 'api' ? [container('taker', 'running', [5432])] : [] + )); + vi.spyOn( + svc as unknown as { regenerateOverridesAcrossFleet: (n?: number, s?: string) => Promise }, + 'regenerateOverridesAcrossFleet', + ).mockResolvedValue(undefined); + await expect(svc.optInStack(localNode.id, 'api', 'tester')) + .rejects.toThrow(`port 5432 is already claimed by ${host}`); + expect(DatabaseService.getInstance().isMeshStackEnabled(localNode.id, 'api')).toBe(false); + } + + it('keeps the saved claim when services is a sequence', async () => { + await expectInvalidDeclarationKeepsClaim('sequence-declaration-stack', 'services:\n - web\n'); + }); + + it('keeps the saved claim when a service definition is not a mapping', async () => { + await expectInvalidDeclarationKeepsClaim('scalar-service-stack', 'services:\n web: busybox\n'); + }); + + it('keeps the saved claim when the services value is a scalar', async () => { + await expectInvalidDeclarationKeepsClaim('scalar-services-stack', 'services: web\n'); + }); + + it('keeps a saved claim when COMPOSE_FILE selects the file chain', async () => { + // compose.yaml declares web, db.yaml declares db, and .env points + // Compose at both. Reading only the discovered main file would think + // db was removed and free its port. + const stack = 'compose-file-chain-stack'; + const localNode = seedClaim(stack, [ + { service: 'db', ports: [5432] }, + { service: 'web', ports: [8080] }, + ]); + const host = `db.${stack}.${localNode.name}.sencho`; + writeComposeFile(stack, ['web']); + writeComposeFile(stack, ['db'], 'db.yaml'); + writeRawComposeFile(stack, 'COMPOSE_FILE=compose.yaml:db.yaml\n', '.env'); + + await withLocalDocker(async () => [container('web', 'running', [8080])]); + const svc = MeshService.getInstance(); + await svc.refreshAliasCache(); + + expect(savedServices(localNode.id, stack)).toEqual([ + { service: 'web', ports: [8080] }, + { service: 'db', ports: [5432] }, + ]); + expect((svc as unknown as Internals).reservedPorts.get(5432)?.host).toBe(host); + }); + + it('keeps a saved claim when the COMPOSE_FILE selection cannot be read', async () => { + // The selector refuses this .env, which means Compose's own discovery + // cannot be trusted either. Unknown keeps the claim. + const stack = 'compose-file-unknown-stack'; + const localNode = seedClaim(stack, [ + { service: 'db', ports: [5432] }, + { service: 'web', ports: [8080] }, + ]); + const host = `db.${stack}.${localNode.name}.sencho`; + writeComposeFile(stack, ['web']); + writeRawComposeFile(stack, 'COMPOSE_DISABLE_ENV_FILE=maybe\n', '.env'); + + await withLocalDocker(async () => [container('web', 'running', [8080])]); + const svc = MeshService.getInstance(); + await svc.refreshAliasCache(); + + expect(savedServices(localNode.id, stack)).toEqual([ + { service: 'web', ports: [8080] }, + { service: 'db', ports: [5432] }, + ]); + expect((svc as unknown as Internals).reservedPorts.get(5432)?.host).toBe(host); + }); + + it('still reconciles a Git file list when COMPOSE_FILE is set', async () => { + // An explicit `-f` list is what Compose loads for a multi-file Git + // stack, so COMPOSE_FILE does not apply. The second file has to stay + // readable, and a service gone from every file still releases its port. + const stack = 'git-file-list-stack'; + const db = DatabaseService.getInstance(); + const localNode = seedClaim(stack, [ + { service: 'db', ports: [5432] }, + { service: 'queue', ports: [6379] }, + { service: 'web', ports: [8080] }, + ]); + writeComposeFile(stack, ['web']); + writeComposeFile(stack, ['queue'], 'queue.yaml'); + writeRawComposeFile(stack, 'COMPOSE_FILE=compose.yaml:queue.yaml\n', '.env'); + vi.spyOn(db, 'getGitSource').mockReturnValue({ + applied_deploy_spec: { files: ['compose.yaml', 'queue.yaml'], contextDir: null }, + } as unknown as ReturnType); + + await withLocalDocker(async () => [container('web', 'running', [8080])]); + const svc = MeshService.getInstance(); + await svc.refreshAliasCache(); + + expect(savedServices(localNode.id, stack)).toEqual([ + { service: 'web', ports: [8080] }, + { service: 'queue', ports: [6379] }, + ]); + const internals = svc as unknown as Internals; + expect(internals.reservedPorts.has(5432)).toBe(false); + expect(internals.reservedPorts.get(6379)?.serviceName).toBe('queue'); + }); + + it.each([ + ['networks-only', 'networks:\n front:\n driver: bridge\n'], + ['volumes-only', 'volumes:\n data:\n'], + ])('releases an authored removal beside a %s auxiliary layer', async (_label, override) => { + // A valid auxiliary layer that only adjusts networks or volumes omits + // `services` on purpose. It contributes no names, and it must not turn + // the rest of the declaration into an unknown that keeps db forever. + const stack = 'auxiliary-layer-stack'; + const localNode = seedClaim(stack, [ + { service: 'db', ports: [5432] }, + { service: 'web', ports: [8080] }, + ]); + writeComposeFile(stack, ['web']); + writeRawComposeFile(stack, override, 'compose.override.yml'); + + await withLocalDocker(async () => [container('web', 'running', [8080])]); + const svc = MeshService.getInstance(); + await svc.refreshAliasCache(); + + expect(savedServices(localNode.id, stack)).toEqual([{ service: 'web', ports: [8080] }]); + const internals = svc as unknown as Internals; + expect(internals.reservedPorts.has(5432)).toBe(false); + expect(internals.reservedPorts.get(8080)?.serviceName).toBe('web'); + }); + + it('keeps all claims when one layer of the chain is malformed', async () => { + // The base file still declares web, but the override is invalid. A + // partial read of the chain cannot release the saved db claim. + const stack = 'malformed-layer-stack'; + const localNode = seedClaim(stack, [ + { service: 'db', ports: [5432] }, + { service: 'web', ports: [8080] }, + ]); + const host = `db.${stack}.${localNode.name}.sencho`; + writeComposeFile(stack, ['web']); + writeRawComposeFile(stack, 'services:\n - db\n', 'compose.override.yml'); + + await withLocalDocker(async () => [container('web', 'running', [8080])]); + const svc = MeshService.getInstance(); + await svc.refreshAliasCache(); + + expect(savedServices(localNode.id, stack)).toEqual([ + { service: 'web', ports: [8080] }, + { service: 'db', ports: [5432] }, + ]); + expect((svc as unknown as Internals).reservedPorts.get(5432)?.host).toBe(host); + }); +}); + /** * Refreshes overlap: the 60 s timer, opt-in/opt-out, and the override-apply * paths all call `refreshAliasCache`. A run that started earlier can finish diff --git a/backend/src/__tests__/mesh-local-services-route.test.ts b/backend/src/__tests__/mesh-local-services-route.test.ts index e05c24c32..7425e0593 100644 --- a/backend/src/__tests__/mesh-local-services-route.test.ts +++ b/backend/src/__tests__/mesh-local-services-route.test.ts @@ -120,4 +120,20 @@ describe('GET /api/mesh/local-declared-services/:stackName', () => { expect(res.status).toBe(503); expect(res.body.error).toMatch(/declaration is unreadable/i); }); + + it('answers 503 when services is not a service mapping', async () => { + // An editor can save invalid content. Returning 200 with the array + // indexes would let central treat "0" as a service name and release a + // real reservation, so the route reports unknown instead. + const dir = path.join(process.env.COMPOSE_DIR as string, 'invalid-declared-stack'); + fsSync.mkdirSync(dir, { recursive: true }); + fsSync.writeFileSync(path.join(dir, 'compose.yaml'), 'services:\n - web\n', 'utf8'); + + const res = await request(app) + .get('/api/mesh/local-declared-services/invalid-declared-stack') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(503); + expect(res.body.error).toMatch(/declaration is unreadable/i); + }); }); diff --git a/backend/src/services/MeshService.ts b/backend/src/services/MeshService.ts index c0d97da2f..296990d45 100644 --- a/backend/src/services/MeshService.ts +++ b/backend/src/services/MeshService.ts @@ -24,6 +24,7 @@ import { isDebugEnabled } from '../utils/debug'; import { safeRemoteFetch } from '../utils/outboundTarget'; import { isPathWithinBase, isValidStackName, isValidRelativeStackPath } from '../utils/validation'; import { getErrorMessage } from '../utils/errors'; +import { composeFileEnvVarSelectsLayers } from '../utils/authoredComposeArgs'; import { PORT as SENCHO_LISTEN_PORT } from '../helpers/constants'; import { assertPolicyGateAllows, buildSystemPolicyGateOptions } from '../helpers/policyGate'; import { @@ -194,6 +195,23 @@ function parseRemoteStackServices(value: unknown): MeshStackService[] | null { return services; } +/** A YAML mapping, as distinct from an array or a scalar. */ +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +/** + * True for a Compose `services` value: a service name to definition mapping, + * where every definition is itself a mapping. Compose rejects a sequence or a + * scalar here, and the stack save route stores raw text, so such content can + * exist on disk. It says nothing about which services the stack still owns, + * so the declaration reader treats it as unknown rather than reading names + * off it. + */ +function isDeclaredServiceMap(value: unknown): value is Record { + return isRecord(value) && Object.values(value).every(isRecord); +} + /** * Returns the static IPv4 address Sencho will pin itself to on the mesh * Docker network: ` + 2`. The Docker daemon assigns @@ -2606,6 +2624,16 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { const baseDir = fsSvc.getBaseDir(); let relFiles: string[]; try { + const spec = DatabaseService.getInstance().getGitSource(stackName)?.applied_deploy_spec; + // An explicit `-f` list is what Compose loads for a Git multi-file + // stack, so COMPOSE_FILE does not change it. A discovered chain is + // replaced by COMPOSE_FILE, which this reader does not follow: the + // files it would read are not the project's files, so the + // declaration is unknown rather than partial. The effective-anatomy + // renderer treats the same condition as unverifiable. + if (!(spec && spec.files.length > 0) && await composeFileEnvVarSelectsLayers(stackName, nodeId)) { + return null; + } relFiles = await this.resolveStackComposeFiles(fsSvc, stackName); // Compose auto-loads the hand-authored override beside a // single-file stack, so a service declared only there is part of @@ -2625,22 +2653,29 @@ export class MeshService extends EventEmitter implements MeshForwarderHost { for (const relFile of relFiles) { const composePath = path.join(baseDir, path.basename(stackName), relFile); if (!isPathWithinBase(composePath, baseDir)) return null; - let parsed: { services?: Record; include?: unknown } | null; + let parsed: unknown; try { - parsed = YAML.parse(await fs.readFile(composePath, 'utf8'), { merge: true }) as { - services?: Record; - include?: unknown; - } | null; + parsed = YAML.parse(await fs.readFile(composePath, 'utf8'), { merge: true }); } catch { return null; } + // An empty layer contributes nothing; it is not a declaration that + // the stack stopped owning every service. + if (parsed === null || parsed === undefined) continue; + if (!isRecord(parsed)) return null; // `include` pulls services in from other files this reader does // not follow. The parsed map is then partial, and a partial // declaration must not release anything. - if (parsed?.include !== undefined) return null; - const services = parsed?.services && typeof parsed.services === 'object' ? parsed.services : null; - if (!services) return null; - for (const name of Object.keys(services)) { + if (parsed.include !== undefined) return null; + // `services` is optional in an auxiliary layer. A file that only + // adjusts networks or volumes is valid, and it contributes no + // service names to the union. + const declaredServices = parsed.services; + if (declaredServices === undefined) continue; + // Anything else is not Compose content. Reading names off an array + // would treat its indexes as services and release real claims. + if (!isDeclaredServiceMap(declaredServices)) return null; + for (const name of Object.keys(declaredServices)) { if (/^[A-Za-z0-9_][A-Za-z0-9_.-]*$/.test(name)) names.add(name); } } diff --git a/docs/features/sencho-mesh.mdx b/docs/features/sencho-mesh.mdx index bb5b4b8a6..1401947a8 100644 --- a/docs/features/sencho-mesh.mdx +++ b/docs/features/sencho-mesh.mdx @@ -70,7 +70,7 @@ Each opt-in and opt-out triggers a redeploy of the affected stack so its `/etc/h **Stack stopped.** The opt-in record is sticky. A stack that opts in and then stops continues to show on the node card with a `suspended` pill and the caption `Stack stopped, alias resumes when services start.` Aliases republish on the next alias-refresh tick (within roughly one minute) when the stack starts again. No need to opt out and back in. -**Node offline.** When Sencho cannot determine a node's current state, it keeps the aliases and ports that node's meshed stacks last published. That covers an offline node and a node that answers with an error, such as after its API token rotates. Other nodes' overrides still list those hostnames (so redeploying a stack elsewhere does not drop them), and no other stack can claim those ports while the node is away. Connections to the node fail until it responds again. Stopped stacks release their aliases but keep their ports reserved, so a stopped stack never frees a port for a different stack to take, and a stack that is mid-teardown during a Take down or update keeps them too. A reservation is released only when the stack stops owning the service: the service runs again without that port, the service leaves the stack's compose file, or you opt the stack out. For a port held by an offline node, the recovery is to wait for the node or remove it from the fleet. +**Node offline.** When Sencho cannot determine a node's current state, it keeps the aliases and ports that node's meshed stacks last published. That covers an offline node and a node that answers with an error, such as after its API token rotates. Other nodes' overrides still list those hostnames (so redeploying a stack elsewhere does not drop them), and no other stack can claim those ports while the node is away. Connections to the node fail until it responds again. Stopped stacks release their aliases but keep their ports reserved, so a stopped stack never frees a port for a different stack to take, and a stack that is mid-teardown during a Take down or update keeps them too. A reservation is released only when Sencho can read the stack's full compose declaration and the service is no longer part of it: the service runs again without that port, it leaves the stack's compose file, or you opt the stack out. When the declaration is incomplete or cannot be read, for example a stack that selects its own compose files with `COMPOSE_FILE`, the reservation stays until you opt the stack out. For a port held by an offline node, the recovery is to wait for the node or remove it from the fleet. **Peer reconnect.** On every alias-refresh tick (60 s), central recomputes the global alias set and pushes refreshed overrides to any node whose set drifted. When a pilot tunnel comes up, central also re-pushes that node's overrides, so the boot race does not leave them stale. Stack redeploys are skipped if the override content is unchanged.