feat: enforce singleton local node per instance (#1567)

Only one local node is allowed. Creating a second local returns 409,
and the last local node cannot be deleted or converted to a remote type.
Existing duplicate local nodes from older versions are preserved and can
be cleaned up individually. Zero-local recovery auto-assigns the default
flag. Frontend delete surfaces and the Add Node form respect the new
invariant.

Enforced in DatabaseService (addNode/updateNode/deleteNode guards) and
routes (error translations). Legacy test fixtures use raw SQL helpers.
This commit is contained in:
Anso
2026-07-05 05:30:56 -04:00
committed by GitHub
parent 33231089c3
commit a7e856f447
11 changed files with 507 additions and 119 deletions
@@ -63,6 +63,15 @@ afterAll(() => {
cleanupTestDb(tmpDir);
});
/** Insert a second local node via raw SQL, bypassing the addNode singleton guard. */
function insertLegacyLocal(name: string): number {
const db = DatabaseService.getInstance().getDb();
const result = db.prepare(
"INSERT INTO nodes (name, type, compose_dir, is_default, status, created_at) VALUES (?, 'local', ?, 0, 'online', ?)"
).run(name, process.env.COMPOSE_DIR ?? '', Date.now());
return result.lastInsertRowid as number;
}
describe('AutoHealService.evaluate', () => {
it('evaluates existing policies on the Community tier (no paid gate)', async () => {
const db = DatabaseService.getInstance();
@@ -318,14 +327,7 @@ describe('AutoHealService.evaluate', () => {
it('evaluates only policies scoped to each local node', async () => {
const db = DatabaseService.getInstance();
const secondNodeId = db.addNode({
name: 'second-local',
type: 'local',
compose_dir: process.env.COMPOSE_DIR ?? '',
is_default: false,
api_url: '',
api_token: '',
});
const secondNodeId = insertLegacyLocal('second-local');
makePolicy(db, { node_id: secondNodeId, stack_name: 'second-stack' });
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('paid');
const getAllContainers = vi.fn().mockResolvedValue([]);
@@ -341,14 +343,7 @@ describe('AutoHealService.evaluate', () => {
it('keeps restart rate-limit state isolated by node while pruning', async () => {
const db = DatabaseService.getInstance();
const secondNodeId = db.addNode({
name: 'rate-limit-second-local',
type: 'local',
compose_dir: process.env.COMPOSE_DIR ?? '',
is_default: false,
api_url: '',
api_token: '',
});
const secondNodeId = insertLegacyLocal('rate-limit-second-local');
const policy = makePolicy(db, { node_id: secondNodeId, stack_name: 'second-rate-stack' });
vi.spyOn(DockerController, 'getInstance').mockReturnValue({
getAllContainers: vi.fn().mockResolvedValue([]),
@@ -40,6 +40,18 @@ afterAll(() => {
cleanupTestDb(tmpDir);
});
/** Insert a second local node via raw SQL, bypassing the addNode singleton guard. */
function insertLegacyLocal(name: string, isDefault = false): number {
const rawDb = db.getDb();
if (isDefault) {
rawDb.prepare('UPDATE nodes SET is_default = 0').run();
}
const result = rawDb.prepare(
"INSERT INTO nodes (name, type, compose_dir, is_default, status, created_at) VALUES (?, 'local', ?, ?, 'online', ?)"
).run(name, process.env.COMPOSE_DIR ?? '', isDefault ? 1 : 0, Date.now());
return result.lastInsertRowid as number;
}
describe('DatabaseService - auto-heal policy CRUD', () => {
it('addAutoHealPolicy + getAutoHealPolicy round-trip preserves all fields', () => {
const input = makePolicy({
@@ -111,14 +123,7 @@ describe('DatabaseService - auto-heal policy CRUD', () => {
it('auto-heal node migration does not rewrite already-scoped node 1 policies', () => {
const created = db.addAutoHealPolicy(makePolicy({ stack_name: 'migration-node-one', node_id: 1 }));
db.addNode({
name: 'new-default-node',
type: 'local',
compose_dir: process.env.COMPOSE_DIR ?? '',
is_default: true,
api_url: '',
api_token: '',
});
insertLegacyLocal('new-default-node', true);
(db as any).migrateAutoHealNodeId();
@@ -128,14 +133,7 @@ describe('DatabaseService - auto-heal policy CRUD', () => {
it('auto-heal node migration resumes backfill when the completion marker is missing', () => {
db.updateGlobalSetting('migration_auto_heal_node_scope_v1', '');
const created = db.addAutoHealPolicy(makePolicy({ stack_name: 'migration-partial', node_id: 1 }));
const newDefaultId = db.addNode({
name: 'partial-new-default-node',
type: 'local',
compose_dir: process.env.COMPOSE_DIR ?? '',
is_default: true,
api_url: '',
api_token: '',
});
const newDefaultId = insertLegacyLocal('partial-new-default-node', true);
(db as any).migrateAutoHealNodeId();
+14 -16
View File
@@ -76,6 +76,18 @@ afterAll(() => {
cleanupTestDb(tmpDir);
});
/** Insert a second local node via raw SQL, bypassing the addNode singleton guard. */
function insertLegacyLocal(name: string, isDefault = false): number {
const db = DatabaseService.getInstance().getDb();
if (isDefault) {
db.prepare('UPDATE nodes SET is_default = 0').run();
}
const result = db.prepare(
"INSERT INTO nodes (name, type, compose_dir, is_default, status, created_at) VALUES (?, 'local', ?, ?, 'online', ?)"
).run(name, process.env.COMPOSE_DIR ?? '', isDefault ? 1 : 0, Date.now());
return result.lastInsertRowid as number;
}
describe('/api/auto-heal routes', () => {
it('allows Community tier access', async () => {
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('community');
@@ -135,14 +147,7 @@ describe('/api/auto-heal routes', () => {
it('lists only policies for the active node', async () => {
const defaultNodeId = DatabaseService.getInstance().getDefaultNode()?.id ?? 1;
const secondNodeId = DatabaseService.getInstance().addNode({
name: 'route-second-local',
type: 'local',
compose_dir: process.env.COMPOSE_DIR ?? '',
is_default: false,
api_url: '',
api_token: '',
});
const secondNodeId = insertLegacyLocal('route-second-local');
makePolicy(defaultNodeId, 'same-stack');
makePolicy(secondNodeId, 'same-stack');
@@ -163,14 +168,7 @@ describe('/api/auto-heal routes', () => {
});
it('rejects history access for a policy owned by a different node', async () => {
const secondNodeId = DatabaseService.getInstance().addNode({
name: 'history-second-local',
type: 'local',
compose_dir: process.env.COMPOSE_DIR ?? '',
is_default: false,
api_url: '',
api_token: '',
});
const secondNodeId = insertLegacyLocal('history-second-local');
const policy = makePolicy(secondNodeId, 'history-stack');
const res = await request(app)
+295
View File
@@ -285,3 +285,298 @@ describe('DELETE /api/nodes/:id default-node guard (M-4)', () => {
expect(res.body.error).toMatch(/default node/i);
});
});
// ---- helpers for singleton tests ----
function getLocalNodeId(): number {
return DatabaseService.getInstance().getNodes().find(n => n.type === 'local')!.id;
}
async function makeRemoteDefault(token: string): Promise<{ remoteId: number; originalDefaultId: number }> {
const list = await request(app).get('/api/nodes').set('Authorization', token);
const originalDefault = (list.body as Array<{ id: number; is_default: boolean }>).find(n => n.is_default)!;
const remoteId = await createRemoteNode(token);
await request(app)
.put(`/api/nodes/${remoteId}`)
.set('Authorization', token)
.send({ is_default: true });
return { remoteId, originalDefaultId: originalDefault.id };
}
async function restoreDefault(token: string, defaultId: number): Promise<void> {
await request(app)
.put(`/api/nodes/${defaultId}`)
.set('Authorization', token)
.send({ is_default: true });
}
/** Insert a second local node via raw SQL, bypassing the addNode singleton guard. */
function insertLegacyLocal(name: string, isDefault = false): number {
const db = DatabaseService.getInstance().getDb();
if (isDefault) {
db.prepare('UPDATE nodes SET is_default = 0').run();
}
const result = db.prepare(
"INSERT INTO nodes (name, type, compose_dir, is_default, status, created_at) VALUES (?, 'local', ?, ?, 'online', ?)"
).run(name, process.env.COMPOSE_DIR ?? '', isDefault ? 1 : 0, Date.now());
return result.lastInsertRowid as number;
}
// ---- singleton enforcement (HTTP layer) ----
describe('Local node singleton enforcement', () => {
it('POST rejects a second local node with 409', async () => {
const res = await request(app)
.post('/api/nodes')
.set('Authorization', authHeader)
.send({ name: 'second-local', type: 'local', compose_dir: '/app/compose' });
expect(res.status).toBe(409);
expect(res.body.error).toMatch(/a local node already exists/i);
});
it('POST local with is_default:true does not clear the existing default when rejected', async () => {
const listBefore = await request(app).get('/api/nodes').set('Authorization', authHeader);
const defBefore = (listBefore.body as Array<{ id: number; is_default: boolean }>).find(n => n.is_default)!;
const res = await request(app)
.post('/api/nodes')
.set('Authorization', authHeader)
.send({ name: 'rejected-local', type: 'local', is_default: true, compose_dir: '/app/compose' });
expect(res.status).toBe(409);
const listAfter = await request(app).get('/api/nodes').set('Authorization', authHeader);
const defAfter = (listAfter.body as Array<{ id: number; is_default: boolean }>).find(n => n.is_default)!;
expect(defAfter.id).toBe(defBefore.id);
});
it('PUT rejects type change from local to remote with 400', async () => {
const localId = getLocalNodeId();
const res = await request(app)
.put(`/api/nodes/${localId}`)
.set('Authorization', authHeader)
.send({ type: 'remote' });
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/cannot be changed/i);
});
it('PUT rejects type change from remote to local with 400', async () => {
const remoteId = await createRemoteNode(authHeader);
const res = await request(app)
.put(`/api/nodes/${remoteId}`)
.set('Authorization', authHeader)
.send({ type: 'local' });
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/cannot be changed/i);
});
it('PUT rejects invalid type value with 400', async () => {
const localId = getLocalNodeId();
const res = await request(app)
.put(`/api/nodes/${localId}`)
.set('Authorization', authHeader)
.send({ type: 'invalid' });
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/must be "local" or "remote"/i);
});
it('PUT allows renaming the local node', async () => {
const localId = getLocalNodeId();
const originalName = DatabaseService.getInstance().getNode(localId)!.name;
try {
const res = await request(app)
.put(`/api/nodes/${localId}`)
.set('Authorization', authHeader)
.send({ name: 'Renamed Local' });
expect(res.status).toBe(200);
expect(DatabaseService.getInstance().getNode(localId)!.name).toBe('Renamed Local');
} finally {
DatabaseService.getInstance().updateNode(localId, { name: originalName });
}
});
it('PUT allows changing compose_dir on the local node', async () => {
const localId = getLocalNodeId();
const originalDir = DatabaseService.getInstance().getNode(localId)!.compose_dir;
try {
const res = await request(app)
.put(`/api/nodes/${localId}`)
.set('Authorization', authHeader)
.send({ compose_dir: '/tmp/test-compose' });
expect(res.status).toBe(200);
expect(DatabaseService.getInstance().getNode(localId)!.compose_dir).toBe('/tmp/test-compose');
} finally {
DatabaseService.getInstance().updateNode(localId, { compose_dir: originalDir });
}
});
it('DELETE rejects the last local node with 400', async () => {
const { remoteId, originalDefaultId } = await makeRemoteDefault(authHeader);
try {
const res = await request(app)
.delete(`/api/nodes/${originalDefaultId}`)
.set('Authorization', authHeader);
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/only local node/i);
} finally {
await restoreDefault(authHeader, originalDefaultId);
const db = DatabaseService.getInstance();
if (db.getNode(remoteId)) db.deleteNode(remoteId);
}
});
it('DELETE allows removing an extra local when more than one exists', async () => {
const extraId = insertLegacyLocal('legacy-extra-local');
try {
const remoteId = await createRemoteNode(authHeader);
await request(app)
.put(`/api/nodes/${remoteId}`)
.set('Authorization', authHeader)
.send({ is_default: true });
try {
const res = await request(app)
.delete(`/api/nodes/${extraId}`)
.set('Authorization', authHeader);
expect(res.status).toBe(200);
expect(DatabaseService.getInstance().getLocalNodeCount()).toBe(1);
} finally {
await restoreDefault(authHeader, getLocalNodeId());
const db = DatabaseService.getInstance();
if (db.getNode(remoteId)) db.deleteNode(remoteId);
}
} finally {
const db = DatabaseService.getInstance();
if (db.getNode(extraId)) db.deleteNode(extraId);
}
});
});
// ---- DatabaseService direct enforcement ----
describe('DatabaseService direct local-node enforcement', () => {
it('addNode throws when a second local is inserted directly', () => {
const db = DatabaseService.getInstance();
expect(() => db.addNode({
name: 'direct-second-local',
type: 'local',
compose_dir: '/app/compose',
is_default: false,
api_url: '',
api_token: '',
})).toThrow(/a local node already exists/i);
});
it('addNode with is_default:true does not clear existing default when it throws', () => {
const db = DatabaseService.getInstance();
const defaultBefore = db.getDefaultNode()!.id;
expect(() => db.addNode({
name: 'direct-rejected-local',
type: 'local',
compose_dir: '/app/compose',
is_default: true,
api_url: '',
api_token: '',
})).toThrow(/a local node already exists/i);
expect(db.getDefaultNode()!.id).toBe(defaultBefore);
});
it('updateNode throws when type is changed', () => {
const db = DatabaseService.getInstance();
const localId = getLocalNodeId();
expect(() => db.updateNode(localId, { type: 'remote' as any })).toThrow(/cannot be changed/i);
});
it('deleteNode throws when the last local is deleted directly', () => {
const db = DatabaseService.getInstance();
const localId = getLocalNodeId();
const remoteId = db.addNode({
name: `direct-remote-${Date.now()}`,
type: 'remote',
mode: 'proxy',
compose_dir: '/app/compose',
is_default: true,
api_url: 'http://192.168.1.77:1852',
api_token: 'tok',
});
try {
expect(() => db.deleteNode(localId)).toThrow(/only local node/i);
} finally {
db.updateNode(localId, { is_default: true });
db.deleteNode(remoteId);
}
});
it('addNode auto-assigns is_default when creating a local during zero-local recovery', () => {
const db = DatabaseService.getInstance();
const localId = getLocalNodeId();
const originalType = db.getNode(localId)!.type;
// Temporarily remove the only local by flipping its type, simulating a
// legacy DB with remotes only.
db.getDb().prepare("UPDATE nodes SET type = 'remote' WHERE id = ?").run(localId);
let newId: number | undefined;
try {
newId = db.addNode({
name: 'recovery-local',
type: 'local',
compose_dir: '/app/compose',
is_default: false,
api_url: '',
api_token: '',
});
expect(db.getNode(newId)!.is_default).toBe(true);
} finally {
// Restore the original local identity. The recovery node was made
// default; re-assign to the original before cleanup.
db.getDb().prepare("UPDATE nodes SET type = ? WHERE id = ?").run(originalType, localId);
if (newId !== undefined && db.getNode(newId)) {
db.updateNode(localId, { is_default: true });
db.deleteNode(newId);
}
}
});
});
// ---- startup warnings ----
describe('logLocalNodeWarnings', () => {
it('warns when there are zero local nodes', () => {
const db = DatabaseService.getInstance();
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
const localId = getLocalNodeId();
const originalType = db.getNode(localId)!.type;
db.getDb().prepare("UPDATE nodes SET type = 'remote' WHERE id = ?").run(localId);
try {
db.logLocalNodeWarnings();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('No local node found'));
} finally {
db.getDb().prepare('UPDATE nodes SET type = ? WHERE id = ?').run(originalType, localId);
}
} finally {
warnSpy.mockRestore();
}
});
it('warns when there are multiple local nodes', () => {
const db = DatabaseService.getInstance();
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
const extraId = insertLegacyLocal('warn-extra-local');
try {
db.logLocalNodeWarnings();
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Found 2 local nodes'));
} finally {
warnSpy.mockRestore();
if (db.getNode(extraId)) db.deleteNode(extraId);
}
});
it('does not warn when exactly one local node exists', () => {
const db = DatabaseService.getInstance();
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
try {
db.logLocalNodeWarnings();
expect(warnSpy).not.toHaveBeenCalled();
} finally {
warnSpy.mockRestore();
}
});
});
+17 -3
View File
@@ -245,6 +245,9 @@ nodesRouter.post('/', enrollmentLimiter, async (req: Request, res: Response) =>
});
} catch (error: unknown) {
const message = error instanceof Error ? error.message : '';
if (message.includes('A local node already exists')) {
return res.status(409).json({ error: message });
}
if (message.includes('UNIQUE constraint')) {
return res.status(409).json({ error: 'A node with that name already exists' });
}
@@ -308,6 +311,10 @@ nodesRouter.put('/:id', async (req: Request, res: Response) => {
updates.compose_dir = composeDir;
}
if (updates.type !== undefined && !['local', 'remote'].includes(updates.type)) {
return res.status(400).json({ error: 'Node type must be "local" or "remote"' });
}
if (updates.api_url !== undefined && updates.api_url !== '') {
const urlCheck = isValidRemoteUrl(updates.api_url);
if (!urlCheck.valid) {
@@ -348,9 +355,12 @@ nodesRouter.put('/:id', async (req: Request, res: Response) => {
})
});
} catch (error: unknown) {
const message = error instanceof Error ? error.message : '';
if (message.includes('Node type cannot be changed')) {
return res.status(400).json({ error: message });
}
console.error('Failed to update node:', error);
const message = error instanceof Error ? error.message : 'Failed to update node';
res.status(500).json({ error: message });
res.status(500).json({ error: message || 'Failed to update node' });
}
});
@@ -384,8 +394,12 @@ nodesRouter.delete('/:id', async (req: Request, res: Response) => {
console.log(`[Nodes] Deleted node ${id} ("${sanitizeForLog(existing.name)}")`);
res.json({ success: true });
} catch (error: unknown) {
const message = error instanceof Error ? error.message : '';
if (message.includes('Cannot delete the only local node')) {
return res.status(400).json({ error: message });
}
console.error('Failed to delete node:', error);
res.status(500).json({ error: error instanceof Error ? error.message : 'Failed to delete node' });
res.status(500).json({ error: message || 'Failed to delete node' });
}
});
+69 -17
View File
@@ -1717,6 +1717,36 @@ export class DatabaseService {
'INSERT INTO nodes (name, type, compose_dir, is_default, status, created_at) VALUES (?, ?, ?, ?, ?, ?)'
).run('Local', 'local', process.env.COMPOSE_DIR || '/app/compose', 1, 'online', Date.now());
}
this.logLocalNodeWarnings();
}
/** Count nodes with type='local'. */
public getLocalNodeCount(): number {
const row = this.db.prepare("SELECT COUNT(*) as count FROM nodes WHERE type = 'local'").get() as { count: number };
return row.count;
}
/**
* Log warnings when the local-node count is not exactly one. Extracted as a
* public method so tests can drive it against known database states without
* re-running the full schema migration.
*/
public logLocalNodeWarnings(): void {
const count = this.getLocalNodeCount();
if (count > 1) {
console.warn(
`[Startup] Found ${count} local nodes (expected 1). ` +
'Extra local nodes can be removed in Settings → Nodes. ' +
'Deleting a local node removes its schedules, labels, dossiers, ' +
'and other node-scoped data; containers on the host are not affected.'
);
} else if (count === 0) {
console.warn(
'[Startup] No local node found. ' +
'Create one in Settings → Nodes to manage this instance\'s Docker engine.'
);
}
}
private migrateAdminToUsersTable(): void {
@@ -3342,24 +3372,38 @@ export class DatabaseService {
}
public addNode(node: Omit<Node, 'id' | 'status' | 'created_at' | 'mode' | 'cordoned' | 'cordoned_at' | 'cordoned_reason'> & { mode?: NodeMode }): number {
if (node.is_default) {
this.db.prepare('UPDATE nodes SET is_default = 0').run();
const isLocal = node.type === 'local';
// Guard against duplicate local nodes before any mutation so a throw
// cannot leave the table in a broken state (e.g. default cleared).
if (isLocal && this.getLocalNodeCount() > 0) {
throw new Error('A local node already exists. Only one local node is allowed per instance.');
}
const crypto = CryptoService.getInstance();
const stmt = this.db.prepare(
'INSERT INTO nodes (name, type, compose_dir, is_default, status, created_at, api_url, api_token, mode) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)'
);
const result = stmt.run(
node.name,
node.type,
node.compose_dir || '/app/compose',
node.is_default ? 1 : 0,
'unknown',
Date.now(),
node.api_url || '',
node.api_token ? crypto.encrypt(node.api_token) : '',
node.mode || 'proxy'
);
// When creating a local node and none exists (zero-local recovery),
// make it the default so documentation remains accurate.
const shouldBeDefault = node.is_default || isLocal;
const runInsert = () => {
if (shouldBeDefault) {
this.db.prepare('UPDATE nodes SET is_default = 0').run();
}
const crypto = CryptoService.getInstance();
const stmt = this.db.prepare(
'INSERT INTO nodes (name, type, compose_dir, is_default, status, created_at, api_url, api_token, mode) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)'
);
return stmt.run(
node.name,
node.type,
node.compose_dir || '/app/compose',
shouldBeDefault ? 1 : 0,
'unknown',
Date.now(),
node.api_url || '',
node.api_token ? crypto.encrypt(node.api_token) : '',
node.mode || 'proxy'
);
};
const result = this.db.transaction(runInsert)();
return result.lastInsertRowid as number;
}
@@ -3367,6 +3411,10 @@ export class DatabaseService {
const node = this.getNode(id);
if (!node) throw new Error(`Node with id ${id} not found`);
if (updates.type !== undefined && updates.type !== node.type) {
throw new Error('Node type cannot be changed after creation.');
}
if (updates.is_default) {
this.db.prepare('UPDATE nodes SET is_default = 0').run();
}
@@ -3396,6 +3444,10 @@ export class DatabaseService {
public deleteNode(id: number): void {
const node = this.getNode(id);
// Protect the last local node regardless of is_default flag.
if (node && node.type === 'local' && this.getLocalNodeCount() <= 1) {
throw new Error('Cannot delete the only local node. Each Sencho instance must retain its local identity.');
}
if (node?.is_default) {
throw new Error('Cannot delete the default node');
}