sec: authenticate theta-agent enrollment; directory + discovery fixes (v1.29.0)

SECURITY

/api/agent/ws authenticated nothing. There was no agent registry, so any
client reaching the SSO could register as a node, publish discovery and
telemetry into the admin view, and receive commands -- including a signed
arbitrary_bash -- addressed to a token it guessed. Tokens were generated
in the BROWSER and never recorded server-side, so there was nothing to
validate against and no way to revoke one.

Agents are now rows in a new Agent table, authenticated by SHA-256 token
hash before the connection is registered or the welcome payload is sent.
Tokens are minted by POST /api/agent/enroll and shown once. Revoke and
rotate drop the live socket immediately. All agent actions are audited.

The Ed25519 command-signing key was generated in the AgentManager
constructor, so it changed on every restart and the public_key pinned in
an agent's agent.yml stopped matching. It now lives in OpenBao at
secret/agent/signing-key; if it cannot be loaded the SSO refuses to send
high-risk commands rather than signing with a key no agent has seen.

DIRECTORY

Agents bind to a host resource instead of being matched by hostname, and
a bound agent's discovery is written onto that resource -- previously the
one source running ON the host contributed nothing to the directory.

The resource tree is collapsible, with state persisted per browser.

DISCOVERY

The Proxmox plugin zipped MACs and IPs from two flat lists by index,
attributing addresses to the wrong NIC on multi-NIC guests. NICs are now
keyed by MAC. Adds an endpoint resource parenting each node, sourceId/
vmid/node identity, container-interface filtering, node IP/MAC, and
offline-node handling.

The reconciler could make a resource its own parent, named hosts after
their MAC address, had a dead isIp() regex (\\. matches a backslash),
merged across kinds, and re-read the whole inventory per resource.

Dockerfile.test-runner never copied nodejs/plugins, so every plugin test
suite failed in CI as "Cannot find module".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nodejs/tests/agent_manager.test.js

@@ -1,11 +1,45 @@
 'use strict';
 
 const crypto = require('crypto');
-const agentManager = require('../utils/agent_manager');
 
-describe('AgentManager PROTOCOL.md v1.1.0 Compliance', () => {
+// In-memory stand-in for OpenBao. The signing key lives at secret/agent/
+// signing-key in production; here we only need it to persist across calls so
+// the "same key every time" property is actually exercised rather than mocked
+// away.
+const mockBaoStore = new Map();
+jest.mock('@simpleworkjs/bao-conf', () => ({
+  get: jest.fn(async (path) => mockBaoStore.get(path) || null),
+  set: jest.fn(async (path, value) => { mockBaoStore.set(path, value); }),
+  request: jest.fn(async () => ({ ok: true, status: 200 }))
+}));
+
+const agentManager = require('../utils/agent_manager');
+const agentKeys = require('../utils/agent_keys');
+
+// The manager is now keyed by enrolled Agent rows rather than by a bare token
+// string, so these use a stub row with the same surface the real model gives:
+// an id, and an update() that records what would be persisted.
+function stubAgent(overrides = {}) {
+  const row = {
+    id: overrides.id || crypto.randomUUID(),
+    name: overrides.name || 'test-agent',
+    resourceId: overrides.resourceId || null,
+    revoked: false,
+    persisted: {},
+    ...overrides
+  };
+  row.update = jest.fn(async (patch) => {
+    Object.assign(row.persisted, patch);
+    Object.assign(row, patch);
+    return row;
+  });
+  return row;
+}
+
+describe('AgentManager PROTOCOL.md v1.2.0 Compliance', () => {
   let mockWs;
   let sentMessages;
+  let agent;
 
   beforeEach(() => {
     sentMessages = [];
@@ -14,23 +48,30 @@ describe('AgentManager PROTOCOL.md v1.1.0 Compliance', () => {
       send: jest.fn((msg) => sentMessages.push(JSON.parse(msg))),
       close: jest.fn()
     };
+    agent = stubAgent();
   });
 
-  test('registers agent and tracks initial connection state', () => {
-    const record = agentManager.registerAgent('test-token-123', mockWs, '192.168.1.100');
-    expect(record.token).toBe('test-token-123');
-    expect(record.ipAddress).toBe('192.168.1.100');
-
-    const agents = agentManager.getConnectedAgents();
-    const found = agents.find(a => a.token === 'test-token-123');
-    expect(found).toBeDefined();
-    expect(found.isOnline).toBe(true);
+  test('registers an agent and reports it as connected', () => {
+    agentManager.registerAgent(agent, mockWs, '192.168.1.100');
+    const state = agentManager.liveState(agent.id);
+    expect(state.connected).toBe(true);
+    expect(state.ipAddress).toBe('192.168.1.100');
+    expect(agentManager.isConnected(agent.id)).toBe(true);
   });
 
-  test('processes discovery payload per PROTOCOL.md v1.1.0 Section 3.1', () => {
-    agentManager.registerAgent('test-token-123', mockWs, '192.168.1.100');
+  // registerAgent must not be async: the WS `message` listener is attached in
+  // the same tick, and `ws` drops events emitted before a listener exists. An
+  // awaited DB write here swallowed every agent's first discovery frame, which
+  // is the one it sends immediately on connect.
+  test('registerAgent is synchronous so no message can be missed', () => {
+    const result = agentManager.registerAgent(agent, mockWs, '10.0.0.1');
+    expect(result).toBeUndefined();
+    expect(agentManager.isConnected(agent.id)).toBe(true);
+  });
 
-    const discoveryPayload = {
+  test('persists discovery to the agent row (Section 3.1)', async () => {
+    agentManager.registerAgent(agent, mockWs, '192.168.1.100');
+    await agentManager.handleDiscovery(agent, {
       hostname: 'node-01.local',
       ip_addresses: ['192.168.1.100', '10.0.0.5'],
       os: 'Ubuntu 24.04 LTS',
@@ -39,41 +80,34 @@ describe('AgentManager PROTOCOL.md v1.1.0 Compliance', () => {
       ram_total_gb: 32.0,
       disk_total_gb: 500.0,
       location: 'dc-chicago-rack-4'
-    };
+    });
 
-    agentManager.handleDiscovery('test-token-123', discoveryPayload);
-
-    const agents = agentManager.getConnectedAgents();
-    const agent = agents.find(a => a.token === 'test-token-123');
-    expect(agent.hostname).toBe('node-01.local');
-    expect(agent.discovery.os).toBe('Ubuntu 24.04 LTS');
-    expect(agent.discovery.ip_addresses).toEqual(['192.168.1.100', '10.0.0.5']);
+    const saved = agent.persisted.lastDiscovery;
+    expect(saved.hostname).toBe('node-01.local');
+    expect(saved.os).toBe('Ubuntu 24.04 LTS');
+    expect(saved.ip_addresses).toEqual(['192.168.1.100', '10.0.0.5']);
+    // Durable, not just in memory: an agent that goes offline keeps its facts.
+    expect(agent.persisted.last_seen).toEqual(expect.any(Number));
   });
 
-  test('processes telemetry payload per PROTOCOL.md v1.1.0 Section 3.2', () => {
-    agentManager.registerAgent('test-token-123', mockWs, '192.168.1.100');
-
-    const telemetryPayload = {
+  test('persists telemetry to the agent row (Section 3.2)', async () => {
+    agentManager.registerAgent(agent, mockWs, '192.168.1.100');
+    await agentManager.handleTelemetry(agent, {
       cpu_usage_percent: 14.5,
       ram_usage_percent: 42.1,
       disk_usage_percent: 68.0,
       zfs_health: 'ONLINE',
       gpu_usage_percent: -1.0,
       timestamp: new Date().toISOString()
-    };
+    });
 
-    agentManager.handleTelemetry('test-token-123', telemetryPayload);
-
-    const agents = agentManager.getConnectedAgents();
-    const agent = agents.find(a => a.token === 'test-token-123');
-    expect(agent.telemetry.cpu_usage_percent).toBe(14.5);
-    expect(agent.telemetry.zfs_health).toBe('ONLINE');
+    expect(agent.persisted.lastTelemetry.cpu_usage_percent).toBe(14.5);
+    expect(agent.persisted.lastTelemetry.zfs_health).toBe('ONLINE');
   });
 
-  test('responds to heartbeat with heartbeat_ack per Section 3.3', () => {
-    agentManager.registerAgent('test-token-123', mockWs, '192.168.1.100');
-
-    agentManager.handleHeartbeat('test-token-123', { timestamp: new Date().toISOString() }, mockWs);
+  test('responds to heartbeat with heartbeat_ack (Section 3.3)', async () => {
+    agentManager.registerAgent(agent, mockWs, '192.168.1.100');
+    await agentManager.handleHeartbeat(agent, { timestamp: new Date().toISOString() }, mockWs);
 
     expect(mockWs.send).toHaveBeenCalled();
     const lastMsg = sentMessages[sentMessages.length - 1];
@@ -81,20 +115,92 @@ describe('AgentManager PROTOCOL.md v1.1.0 Compliance', () => {
     expect(lastMsg.payload.timestamp).toBeDefined();
   });
 
-  test('canonicalizes payload and signs high-risk commands using Ed25519 per Section 5', () => {
-    agentManager.registerAgent('test-token-123', mockWs, '192.168.1.100');
+  test('canonicalizes and signs high-risk commands with Ed25519 (Section 5)', async () => {
+    agentManager.registerAgent(agent, mockWs, '192.168.1.100');
 
     const rawPayload = { script: 'uptime', location: 'datacenter' };
-    const msg = agentManager.sendCommand('test-token-123', 'arbitrary_bash', rawPayload, true);
+    const msg = await agentManager.sendCommand(agent, 'arbitrary_bash', rawPayload, true);
 
     expect(msg.type).toBe('arbitrary_bash');
-    expect(msg.payload.signature).toBeDefined();
     expect(typeof msg.payload.signature).toBe('string');
 
-    // Verify signature with public key
-    const signatureBuffer = Buffer.from(msg.payload.signature, 'base64');
-    const canonicalStr = agentManager.canonicalize(rawPayload);
-    const isValid = crypto.verify(null, Buffer.from(canonicalStr, 'utf8'), agentManager.publicKeyPem, signatureBuffer);
+    const keys = await agentKeys.load();
+    const isValid = crypto.verify(
+      null,
+      Buffer.from(agentManager.canonicalize(rawPayload), 'utf8'),
+      crypto.createPublicKey(keys.publicKeyPem),
+      Buffer.from(msg.payload.signature, 'base64')
+    );
     expect(isValid).toBe(true);
   });
+
+  // The canonical form has to match the Go agent's byte for byte. Go's
+  // encoding/json escapes <, > and & by default and JSON.stringify does not, so
+  // the agent uses SetEscapeHTML(false); this pins the server's half of that
+  // contract. See theta-agent TestCanonicalizeMatchesServerForm.
+  test('canonical form is sorted, unescaped, and omits the signature', () => {
+    const canonical = agentManager.canonicalize({
+      script: 'echo a > b && c',
+      comment: 'x&y',
+      signature: 'should-not-appear'
+    });
+    expect(canonical).toBe('{"comment":"x&y","script":"echo a > b && c"}');
+  });
+
+  test('refuses to send to an agent that is not connected', async () => {
+    await expect(agentManager.sendCommand(agent, 'reload_config', {}, false))
+      .rejects.toThrow(/not connected/);
+  });
+
+  // Revocation that only applies on the next reconnect is not revocation.
+  test('disconnect drops the live socket immediately', () => {
+    agentManager.registerAgent(agent, mockWs, '192.168.1.100');
+    expect(agentManager.isConnected(agent.id)).toBe(true);
+
+    const dropped = agentManager.disconnect(agent.id, 4003, 'Enrollment revoked');
+    expect(dropped).toBe(true);
+    expect(mockWs.close).toHaveBeenCalledWith(4003, 'Enrollment revoked');
+    expect(agentManager.isConnected(agent.id)).toBe(false);
+  });
+
+  test('a second connection for the same agent supersedes the first', () => {
+    agentManager.registerAgent(agent, mockWs, '192.168.1.100');
+    const secondWs = { readyState: 1, send: jest.fn(), close: jest.fn() };
+    agentManager.registerAgent(agent, secondWs, '192.168.1.101');
+
+    expect(mockWs.close).toHaveBeenCalledWith(4002, 'Superseded by new connection');
+    expect(agentManager.liveState(agent.id).ipAddress).toBe('192.168.1.101');
+  });
+
+  test('an unknown agent id is simply not connected', () => {
+    expect(agentManager.isConnected('no-such-agent')).toBe(false);
+    expect(agentManager.liveState('no-such-agent')).toEqual({ connected: false, lastResponse: null });
+  });
+});
+
+describe('agent signing key', () => {
+  // The old manager generated a key pair in its constructor, so it changed on
+  // every restart and the public_key pinned in agent.yml stopped matching.
+  test('the same key is returned across repeated loads', async () => {
+    const first = await agentKeys.load();
+    const second = await agentKeys.load();
+    expect(first.publicKeyBase64).toBe(second.publicKeyBase64);
+  });
+
+  test('the exported public key is the raw 32 bytes agents pin', async () => {
+    const keys = await agentKeys.load();
+    expect(Buffer.from(keys.publicKeyBase64, 'base64')).toHaveLength(32);
+  });
+
+  test('rawPublicKeyBase64 strips the SPKI wrapper', () => {
+    const { publicKey } = crypto.generateKeyPairSync('ed25519', {
+      privateKeyEncoding: { type: 'pkcs8', format: 'pem' },
+      publicKeyEncoding: { type: 'spki', format: 'pem' }
+    });
+    const raw = Buffer.from(agentKeys.rawPublicKeyBase64(publicKey), 'base64');
+    expect(raw).toHaveLength(32);
+    // and it is the tail of the DER encoding
+    const der = crypto.createPublicKey(publicKey).export({ type: 'spki', format: 'der' });
+    expect(raw.equals(der.subarray(der.length - 32))).toBe(true);
+  });
 });

nodejs/tests/discovery_naming.test.js

@@ -0,0 +1,104 @@
+'use strict';
+
+// Pure-logic coverage for the two reconciler rules that real Proxmox + UniFi
+// data broke. Both were found by running discovery against a live cluster:
+// the directory came back listing MAC addresses as host names, and one
+// resource ended up as its own parent.
+
+// Mirrors the ranking in services/discovery_reconciler.js. Kept here (rather
+// than exported) because it is a few lines of predicate that the reconciler
+// applies inline while merging; if it grows, export it and drop this copy.
+const isIp = (str) => /^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$/.test(str || '');
+const isMac = (str) => /^([0-9a-f]{2}[:-]){5}[0-9a-f]{2}$/i.test((str || '').trim());
+const nameRank = (str) => {
+	if (!str || !String(str).trim()) return 0;
+	if (isMac(str)) return 0;
+	if (isIp(str)) return 1;
+	return 2;
+};
+function bestNameOf(existingName, incomingName) {
+	let best = existingName;
+	if (incomingName) {
+		const a = nameRank(incomingName);
+		const b = nameRank(best);
+		if (a > b || (a === b && incomingName.length > (best || '').length)) best = incomingName;
+	}
+	return best;
+}
+
+describe('discovery name ranking', () => {
+	test('a real hostname beats a MAC even when shorter', () => {
+		// The exact regression: UniFi named the host by MAC, Proxmox knew the
+		// hostname, and length-only comparison kept the MAC.
+		expect(bestNameOf('ac:16:2d:b3:da:80', 'dl380-0')).toBe('dl380-0');
+	});
+
+	test('a MAC never displaces a real hostname', () => {
+		expect(bestNameOf('dl380-0', 'ac:16:2d:b3:da:80')).toBe('dl380-0');
+	});
+
+	test('a real hostname beats an IP-shaped name', () => {
+		expect(bestNameOf('192.168.1.27', 'hass.io')).toBe('hass.io');
+	});
+
+	test('an IP beats a MAC', () => {
+		expect(bestNameOf('bc:24:11:3f:cd:c8', '192.168.1.27')).toBe('192.168.1.27');
+	});
+
+	test('an IP does not displace a hostname', () => {
+		expect(bestNameOf('gitea-runner', '192.168.1.176')).toBe('gitea-runner');
+	});
+
+	test('within the same rank the longer/more specific name wins', () => {
+		expect(bestNameOf('pve', 'pve-dl380-1')).toBe('pve-dl380-1');
+	});
+
+	test('dash-separated MACs are recognized too', () => {
+		expect(bestNameOf('ac-16-2d-b3-da-80', 'dl380-0')).toBe('dl380-0');
+	});
+
+	test('an empty existing name is always replaced', () => {
+		expect(bestNameOf('', 'anything')).toBe('anything');
+		expect(bestNameOf(null, 'ac:16:2d:b3:da:80')).toBe('ac:16:2d:b3:da:80');
+	});
+});
+
+// Mirrors isDescendant() in the reconciler.
+function isDescendant(candidateId, rootId, edges) {
+	const seen = new Set();
+	const stack = [rootId];
+	while (stack.length) {
+		const id = stack.pop();
+		if (id === candidateId) return true;
+		if (seen.has(id)) continue;
+		seen.add(id);
+		for (const e of edges) if (e.parentId === id) stack.push(e.childId);
+	}
+	return false;
+}
+
+describe('discovery edge cycle guard', () => {
+	const edges = [
+		{ parentId: 'cluster', childId: 'node1' },
+		{ parentId: 'node1', childId: 'vm1' },
+	];
+
+	test('detects a direct parent/child inversion', () => {
+		// Proposing node1 -> cluster when cluster -> node1 already exists.
+		expect(isDescendant('node1', 'cluster', edges)).toBe(true);
+	});
+
+	test('detects a deeper loop', () => {
+		expect(isDescendant('vm1', 'cluster', edges)).toBe(true);
+	});
+
+	test('allows an unrelated new parent', () => {
+		expect(isDescendant('node2', 'cluster', edges)).toBe(false);
+	});
+
+	test('terminates on a graph that already contains a cycle', () => {
+		// A self-edge written by an earlier release must not hang the walk.
+		const cyclic = [{ parentId: 'a', childId: 'a' }, { parentId: 'a', childId: 'b' }];
+		expect(isDescendant('zzz', 'a', cyclic)).toBe(false);
+	});
+});

nodejs/tests/proxmox_interfaces.test.js

@@ -0,0 +1,78 @@
+'use strict';
+
+const { _Interfaces: Interfaces } = require('../plugins/discovery/proxmox');
+
+// Regression coverage for the MAC/IP mismatch: the plugin used to collect MACs
+// and IPs into two flat lists and zip them by index, so on a multi-NIC guest
+// -- or any guest where one NIC had no address -- the directory recorded an IP
+// against the wrong MAC. Interfaces keys by MAC so a pairing can only come from
+// the source that observed both together.
+describe('proxmox Interfaces', () => {
+	test('keeps each IP on the NIC it was observed on', () => {
+		const i = new Interfaces();
+		i.add('AA:BB:CC:00:00:01', ['10.0.0.5'], 'eth0');
+		i.add('AA:BB:CC:00:00:02', ['192.168.9.7'], 'eth1');
+
+		expect(i.toArray()).toEqual([
+			{ mac: 'aa:bb:cc:00:00:01', ip: '10.0.0.5', ips: ['10.0.0.5'], name: 'eth0' },
+			{ mac: 'aa:bb:cc:00:00:02', ip: '192.168.9.7', ips: ['192.168.9.7'], name: 'eth1' },
+		]);
+	});
+
+	test('a NIC with no address does not steal the next NIC\'s IP', () => {
+		const i = new Interfaces();
+		i.add('AA:BB:CC:00:00:01', [], 'eth0');      // stopped/unconfigured
+		i.add('AA:BB:CC:00:00:02', ['10.0.0.9'], 'eth1');
+
+		const byMac = Object.fromEntries(i.toArray().map(x => [x.mac, x.ip]));
+		expect(byMac['aa:bb:cc:00:00:01']).toBeNull();
+		expect(byMac['aa:bb:cc:00:00:02']).toBe('10.0.0.9');
+	});
+
+	test('merges the config MAC with the agent-reported address for the same NIC', () => {
+		const i = new Interfaces();
+		i.add('aa:bb:cc:00:00:01', ['10.0.0.5'], 'eth0'); // guest agent
+		i.add('AA:BB:CC:00:00:01', [], 'net0');           // VM config, same NIC
+		expect(i.toArray()).toHaveLength(1);
+		expect(i.toArray()[0]).toMatchObject({ mac: 'aa:bb:cc:00:00:01', ip: '10.0.0.5' });
+	});
+
+	test('collects multiple addresses on one NIC without inventing a second NIC', () => {
+		const i = new Interfaces();
+		i.add('aa:bb:cc:00:00:01', ['10.0.0.5', '10.0.0.6'], 'eth0');
+		expect(i.toArray()).toHaveLength(1);
+		expect(i.toArray()[0].ips).toEqual(['10.0.0.5', '10.0.0.6']);
+		expect(i.primaryIp()).toBe('10.0.0.5');
+	});
+
+	test('ignores placeholder and malformed MACs', () => {
+		const i = new Interfaces();
+		i.add('00:00:00:00:00:00', [], 'eth0');
+		i.add('not-a-mac', [], 'eth1');
+		i.add('', [], 'eth2');
+		expect(i.toArray()).toEqual([]);
+		expect(i.primaryMac()).toBeNull();
+	});
+
+	test('keeps an address that arrived without a usable MAC', () => {
+		const i = new Interfaces();
+		i.add(null, ['10.0.0.5'], 'eth0');
+		expect(i.primaryIp()).toBe('10.0.0.5');
+		expect(i.primaryMac()).toBeNull();
+	});
+
+	test('primary values prefer a NIC that actually has an address', () => {
+		const i = new Interfaces();
+		i.add('aa:bb:cc:00:00:01', [], 'eth0');
+		i.add('aa:bb:cc:00:00:02', ['10.0.0.9'], 'eth1');
+		expect(i.primaryIp()).toBe('10.0.0.9');
+		expect(i.primaryMac()).toBe('aa:bb:cc:00:00:02');
+	});
+
+	test('a fully unaddressed guest still reports its MAC', () => {
+		const i = new Interfaces();
+		i.add('aa:bb:cc:00:00:01', [], 'net0');
+		expect(i.primaryIp()).toBeNull();
+		expect(i.primaryMac()).toBe('aa:bb:cc:00:00:01');
+	});
+});