From 66ac59af49266175d9da8fe860006c53823d3283 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 16 Jul 2026 05:36:53 -0500 Subject: [PATCH] fix(fleet): validate remote migration targets Co-Authored-By: Claude Opus 4.8 --- .../mosaic/src/fleet/v1-v2-migration.spec.ts | 270 +++++++++++++++++- packages/mosaic/src/fleet/v1-v2-migration.ts | 76 +++-- 2 files changed, 307 insertions(+), 39 deletions(-) diff --git a/packages/mosaic/src/fleet/v1-v2-migration.spec.ts b/packages/mosaic/src/fleet/v1-v2-migration.spec.ts index 544610b..747cffe 100644 --- a/packages/mosaic/src/fleet/v1-v2-migration.spec.ts +++ b/packages/mosaic/src/fleet/v1-v2-migration.spec.ts @@ -75,7 +75,7 @@ agents: class: reviewer host: build.example.test ssh: operator@build.example.test - socket: remote-fleet + socket: mosaic-v1 connector: kind: discord discord: @@ -253,7 +253,7 @@ describe('v1-to-v2 migration preview', (): void => { { label: 'host different from reviewed fleet host', agentFields: - ' host: build.example.test\n ssh: operator@build.example.test\n socket: remote-fleet', + ' host: build.example.test\n ssh: operator@build.example.test\n socket: mosaic-v1', fleetHost: 'fleet.example.test', expectedStatus: 'ready', expectedInventoryOnly: true, @@ -288,7 +288,7 @@ describe('v1-to-v2 migration preview', (): void => { 'classifies $label without inferring locality', async ({ agentFields, fleetHost, expectedStatus, expectedInventoryOnly, expectedBlocker }) => { const source = v1Source.replace( - ' host: build.example.test\n ssh: operator@build.example.test\n socket: remote-fleet', + ' host: build.example.test\n ssh: operator@build.example.test\n socket: mosaic-v1', agentFields, ); const migrationDecisions = decisions(); @@ -358,10 +358,9 @@ describe('v1-to-v2 migration preview', (): void => { ] as const)( 'preserves an explicit empty tmux.%s declaration as the literal default server without a decision', async (_field, tmuxBlock) => { - const source = v1Source.replace( - ' socket_name: mosaic-v1\n holder_session: _holder', - tmuxBlock, - ); + const source = v1Source + .replace(' socket_name: mosaic-v1\n holder_session: _holder', tmuxBlock) + .replace(' socket: mosaic-v1\n', ''); const preview = await previewV1ToV2Migration({ source, decisions: decisions(), @@ -494,7 +493,8 @@ describe('v1-to-v2 migration preview', (): void => { it('blocks a socket-only local agent that conflicts with the canonical fleet socket', async () => { const source = v1Source .replace(' host: build.example.test\n', '') - .replace(' ssh: operator@build.example.test\n', ''); + .replace(' ssh: operator@build.example.test\n', '') + .replace(' socket: mosaic-v1', ' socket: other-socket'); const migrationDecisions = decisions(); migrationDecisions.agents.remote0 = { provider: 'openai', @@ -615,7 +615,7 @@ describe('v1-to-v2 migration preview', (): void => { class: reviewer host: review.example.test ssh: operator@review.example.test - socket: review-fleet + socket: mosaic-v1 `; const sourceA = v1Source.replace('connector:', `${remote1}connector:`); const sourceB = sourceA.replace( @@ -1200,6 +1200,258 @@ describe('v1-to-v2 migration preview', (): void => { expect(JSON.stringify(preview.blockers)).not.toContain(rejectedKind); }); + it.each([ + ['leading whitespace', ' mosaic-v1'], + ['trailing whitespace', 'mosaic-v1 '], + ['unsupported characters', 'mosaic/v1'], + ] as const)( + 'blocks a declared root socket with %s before migration decisions can normalize it', + async (_label, rejectedSocket) => { + const source = v1Source.replace( + 'socket_name: mosaic-v1', + `socket_name: ${JSON.stringify(rejectedSocket)}`, + ); + const rosterPath = join( + await mkdtemp(join(tmpdir(), 'v1-root-socket-parity-')), + 'roster.yaml', + ); + temporaryDirectories.push(dirname(rosterPath)); + await writeFile(rosterPath, source); + await expect(loadFleetRoster(rosterPath)).rejects.toThrow( + 'contains unsupported targeting characters', + ); + + const preview = await previewV1ToV2Migration({ + source, + decisions: { ...decisions(), socketName: 'mosaic-v1' }, + observations: observations(), + personaDirs: await semanticDirs(), + }); + + expect(preview.status).toBe('blocked'); + expect(preview.blockers).toContainEqual({ + code: 'invalid-v1-targeting-characters', + path: 'tmux.socket_name', + detail: 'Fleet tmux socket contains unsupported targeting characters.', + }); + expect(preview).not.toHaveProperty('canonicalRoster'); + expect(preview).not.toHaveProperty('canonicalYaml'); + expect(preview.evidence).not.toHaveProperty('candidate'); + expect(JSON.stringify(preview.blockers)).not.toContain(rejectedSocket); + }, + ); + + it('blocks an explicit remote socket against the source default before a decision can rename the candidate socket', async () => { + const rejectedSocket = 'mosaic-v1'; + const source = v1Source.replace(' socket_name: mosaic-v1\n', ''); + const rosterPath = join(await mkdtemp(join(tmpdir(), 'v1-root-socket-parity-')), 'roster.yaml'); + temporaryDirectories.push(dirname(rosterPath)); + await writeFile(rosterPath, source); + await expect(loadFleetRoster(rosterPath)).rejects.toThrow( + 'socket must equal the fleet-wide tmux socket_name', + ); + + const preview = await previewV1ToV2Migration({ + source, + decisions: { ...decisions(), socketName: rejectedSocket }, + observations: observations(), + personaDirs: await semanticDirs(), + }); + + expect(preview.status).toBe('blocked'); + expect(preview.blockers).toContainEqual({ + code: 'agent-socket-disposition-required', + path: 'agents[1].socket', + agent: 'remote0', + detail: + 'Agent socket must match the canonical v1 fleet socket; independent per-agent sockets are not supported.', + }); + expect(preview).not.toHaveProperty('canonicalRoster'); + expect(preview).not.toHaveProperty('canonicalYaml'); + expect(preview.evidence).not.toHaveProperty('candidate'); + expect(JSON.stringify(preview.blockers)).not.toContain(rejectedSocket); + }); + + it('uses the production empty default when both the source root and remote agent socket are absent', async () => { + const source = v1Source + .replace(' socket_name: mosaic-v1\n', '') + .replace(' socket: mosaic-v1\n', ''); + const rosterPath = join(await mkdtemp(join(tmpdir(), 'v1-root-socket-parity-')), 'roster.yaml'); + temporaryDirectories.push(dirname(rosterPath)); + await writeFile(rosterPath, source); + await expect(loadFleetRoster(rosterPath)).resolves.toMatchObject({ + tmux: { socketName: '' }, + }); + + const preview = await previewV1ToV2Migration({ + source, + decisions: decisions(), + observations: observations(), + personaDirs: await semanticDirs(), + }); + + expect(preview.status).toBe('ready'); + expect(preview.blockers).toEqual([]); + expect(preview.inventoryOnlyAgents).toEqual([ + { + name: 'remote0', + fields: ['host', 'ssh'], + disposition: 'inventory-only', + }, + ]); + expect(preview.canonicalRoster?.tmux.socketName).toBe(''); + expect(preview.canonicalRoster?.agents.map(({ name }) => name)).toEqual(['coder0']); + expect(preview.evidence.candidate).toBeDefined(); + }); + + it('blocks a remote inventory-only agent socket that differs from the root socket', async () => { + const rejectedSocket = 'remote-fleet'; + const source = v1Source.replace(' socket: mosaic-v1', ` socket: ${rejectedSocket}`); + const preview = await previewV1ToV2Migration({ + source, + decisions: decisions(), + observations: observations(), + personaDirs: await semanticDirs(), + }); + + expect(preview.status).toBe('blocked'); + expect(preview.blockers).toEqual([ + { + code: 'agent-socket-disposition-required', + path: 'agents[1].socket', + agent: 'remote0', + detail: + 'Agent socket must match the canonical v1 fleet socket; independent per-agent sockets are not supported.', + }, + ]); + expect(preview.inventoryOnlyAgents).toEqual([ + { + name: 'remote0', + fields: ['host', 'socket', 'ssh'], + disposition: 'inventory-only', + }, + ]); + expect(preview).not.toHaveProperty('canonicalRoster'); + expect(preview).not.toHaveProperty('canonicalYaml'); + expect(preview.evidence).not.toHaveProperty('candidate'); + expect(JSON.stringify(preview.blockers)).not.toContain(rejectedSocket); + }); + + it('keeps a remote agent without an explicit socket inventory-only on the default tmux socket', async () => { + const source = v1Source + .replace(' socket_name: mosaic-v1\n', " socket_name: ''\n") + .replace(' socket: mosaic-v1\n', ''); + const preview = await previewV1ToV2Migration({ + source, + decisions: decisions(), + observations: observations(), + personaDirs: await semanticDirs(), + }); + + expect(preview.status).toBe('ready'); + expect(preview.blockers).toEqual([]); + expect(preview.inventoryOnlyAgents).toEqual([ + { + name: 'remote0', + fields: ['host', 'ssh'], + disposition: 'inventory-only', + }, + ]); + expect(preview.canonicalRoster?.tmux.socketName).toBe(''); + expect(preview.canonicalRoster?.agents.map(({ name }) => name)).toEqual(['coder0']); + expect(preview.evidence.candidate).toBeDefined(); + }); + + it.each([ + ['host', 'bad host'], + ['ssh', 'bad user@remote.test'], + ['socket', 'bad socket'], + ['host', ' build.example.test'], + ['ssh', 'operator@build.example.test '], + ['socket', ' remote-fleet'], + ['host', ''], + ['ssh', ''], + ['socket', ''], + ] as const)( + 'blocks malformed remote %s targeting before inventory-only exclusion', + async (field, rejectedValue) => { + const source = v1Source.replace( + new RegExp(`(${field}: )[^\\n]+`), + `$1${JSON.stringify(rejectedValue)}`, + ); + const productionSource = source; + const rosterPath = join(await mkdtemp(join(tmpdir(), 'v1-targeting-parity-')), 'roster.yaml'); + temporaryDirectories.push(dirname(rosterPath)); + await writeFile(rosterPath, productionSource); + await expect(loadFleetRoster(rosterPath)).rejects.toThrow( + 'contains unsupported targeting characters', + ); + const preview = await previewV1ToV2Migration({ + source, + decisions: decisions(), + observations: observations(), + personaDirs: await semanticDirs(), + }); + + expect(preview.status).toBe('blocked'); + expect(preview.blockers).toContainEqual( + expect.objectContaining({ + code: 'invalid-v1-targeting-characters', + path: `agents[1].${field}`, + agent: 'remote0', + }), + ); + expect(preview).not.toHaveProperty('canonicalRoster'); + expect(preview).not.toHaveProperty('canonicalYaml'); + expect(preview.evidence).not.toHaveProperty('candidate'); + if (rejectedValue) { + expect(JSON.stringify(preview.blockers)).not.toContain(rejectedValue); + } + }, + ); + + it('accepts the production v1 targeting character boundaries for a remote inventory-only agent', async () => { + const source = v1Source + .replace('host: build.example.test', 'host: node-1.example.test:[2001:db8::1]') + .replace( + 'ssh: operator@build.example.test', + 'ssh: user.name-1@node-1.example.test:[2001:db8::1]', + ) + .replace('socket: mosaic-v1', 'socket: mosaic-v1'); + const rosterPath = join(await mkdtemp(join(tmpdir(), 'v1-targeting-parity-')), 'roster.yaml'); + temporaryDirectories.push(dirname(rosterPath)); + await writeFile(rosterPath, source); + await expect(loadFleetRoster(rosterPath)).resolves.toMatchObject({ + agents: [ + expect.objectContaining({ name: 'coder0' }), + expect.objectContaining({ + name: 'remote0', + host: 'node-1.example.test:[2001:db8::1]', + ssh: 'user.name-1@node-1.example.test:[2001:db8::1]', + socket: 'mosaic-v1', + }), + ], + }); + const preview = await previewV1ToV2Migration({ + source, + decisions: decisions(), + observations: observations(), + personaDirs: await semanticDirs(), + }); + + expect(preview.status).toBe('ready'); + expect(preview.blockers).toEqual([]); + expect(preview.inventoryOnlyAgents).toEqual([ + { + name: 'remote0', + fields: ['host', 'socket', 'ssh'], + disposition: 'inventory-only', + }, + ]); + expect(preview.canonicalRoster?.agents.map(({ name }) => name)).toEqual(['coder0']); + expect(preview.evidence.excludedInventory).toContain('agents.remote0'); + }); + it('blocks malformed schema-only remote and connector fields', async () => { const source = v1Source .replace('host: build.example.test', 'host: 42') diff --git a/packages/mosaic/src/fleet/v1-v2-migration.ts b/packages/mosaic/src/fleet/v1-v2-migration.ts index d1bd00f..5e2e4c2 100644 --- a/packages/mosaic/src/fleet/v1-v2-migration.ts +++ b/packages/mosaic/src/fleet/v1-v2-migration.ts @@ -74,6 +74,14 @@ const AGENT_DECISION_FIELDS = new Set([ const CLASS_DISPOSITION_FIELDS = new Set(['action', 'className']); const OBSERVATION_FIELDS = new Set(['systemd', 'tmux']); const AGENT_NAME_PATTERN = /^[A-Za-z0-9_.-]+$/; +const V1_TMUX_SOCKET_PATTERN = /^[A-Za-z0-9_.-]+$/; +const V1_REMOTE_TARGET_PATTERNS: Readonly< + Record<(typeof REMOTE_AGENT_INVENTORY_FIELDS)[number], RegExp> +> = Object.freeze({ + host: /^[A-Za-z0-9_.:[\]-]+$/, + ssh: /^(?:[A-Za-z0-9._-]+@)?[A-Za-z0-9_.:[\]-]+$/, + socket: V1_TMUX_SOCKET_PATTERN, +}); const AGENT_FIELDS = new Set([ 'name', 'alias', @@ -347,16 +355,23 @@ export async function previewV1ToV2Migration( const hasDeclaredSnakeSocket = Object.hasOwn(tmux, 'socket_name'); const hasDeclaredCamelSocket = Object.hasOwn(tmux, 'socketName'); const hasDeclaredSocket = hasDeclaredSnakeSocket || hasDeclaredCamelSocket; - const declaredSocketName = hasDeclaredSnakeSocket - ? trimmedString(tmux.socket_name) - : hasDeclaredCamelSocket - ? trimmedString(tmux.socketName) - : undefined; - const socketName = hasDeclaredSocket ? declaredSocketName : decisions.socketName; + const declaredSocketValue = hasDeclaredSnakeSocket ? tmux.socket_name : tmux.socketName; + const sourceSocketName = typeof declaredSocketValue === 'string' ? declaredSocketValue : ''; + const declaredSocketPath = hasDeclaredSnakeSocket ? 'tmux.socket_name' : 'tmux.socketName'; + if (sourceSocketName && !V1_TMUX_SOCKET_PATTERN.test(sourceSocketName)) { + blockers.push( + blocker( + 'invalid-v1-targeting-characters', + declaredSocketPath, + 'Fleet tmux socket contains unsupported targeting characters.', + ), + ); + } + const socketName = sourceSocketName; if ( hasDeclaredSocket && decisions.socketName !== undefined && - decisions.socketName !== declaredSocketName + decisions.socketName !== sourceSocketName ) { blockers.push( blocker( @@ -377,6 +392,17 @@ export async function previewV1ToV2Migration( blockers.push(blocker('invalid-agent-name', `${path}.name`, 'Agent name is required.')); continue; } + const agentSocket = typeof rawAgent.socket === 'string' ? rawAgent.socket : undefined; + if (agentSocket !== undefined && agentSocket !== socketName) { + blockers.push( + blocker( + 'agent-socket-disposition-required', + `${path}.socket`, + 'Agent socket must match the canonical v1 fleet socket; independent per-agent sockets are not supported.', + name, + ), + ); + } const locality = classifyAgentLocality(rawAgent, decisions.fleetHost); const localityFields = REMOTE_AGENT_INVENTORY_FIELDS.filter( (field) => rawAgent[field] !== undefined, @@ -399,17 +425,6 @@ export async function previewV1ToV2Migration( ), ); } - const agentSocket = stringValue(rawAgent.socket); - if (agentSocket && (!socketName || agentSocket !== socketName)) { - blockers.push( - blocker( - 'agent-socket-disposition-required', - `${path}.socket`, - 'Local agent socket must match the canonical v2 fleet socket or receive an explicit future disposition.', - name, - ), - ); - } const decision = decisions.agents[name]; if (!decision) { @@ -560,15 +575,6 @@ export async function previewV1ToV2Migration( ), ); } - if (socketName === undefined) { - blockers.push( - blocker( - 'named-socket-disposition-required', - 'tmux.socket_name', - 'A socket decision is required only when both supported v1 socket fields are absent.', - ), - ); - } if (candidateAgents.length === 0) { blockers.push( blocker('no-local-candidates', 'agents', 'No local agents are eligible for v2 output.'), @@ -579,7 +585,7 @@ export async function previewV1ToV2Migration( let canonicalYaml: string | undefined; const environment: EnvironmentMigrationPreview[] = []; - if (blockers.length === 0 && socketName !== undefined && decisions.defaultRuntime) { + if (blockers.length === 0 && decisions.defaultRuntime) { const defaultRuntime = decisions.defaultRuntime; try { canonicalRoster = parseRosterV2( @@ -966,12 +972,22 @@ function validateV1FieldCatalog(raw: Record, blockers: Migratio ); } for (const key of REMOTE_AGENT_INVENTORY_FIELDS) { - if (agent[key] !== undefined && stringValue(agent[key]) === undefined) { + const target = agent[key]; + if (target !== undefined && typeof target !== 'string') { blockers.push( blocker( 'invalid-v1-field-type', `${path}.${key}`, - 'Remote inventory field must be a non-empty string.', + 'Remote inventory field must be a string.', + ), + ); + } else if (typeof target === 'string' && !V1_REMOTE_TARGET_PATTERNS[key].test(target)) { + blockers.push( + blocker( + 'invalid-v1-targeting-characters', + `${path}.${key}`, + 'Remote inventory field contains unsupported targeting characters.', + name, ), ); }