fix(fleet): validate remote migration targets
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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')
|
||||
|
||||
@@ -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<string, unknown>, 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,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user