diff --git a/backend/src/__tests__/node-management-hardening.test.ts b/backend/src/__tests__/node-management-hardening.test.ts new file mode 100644 index 00000000..71b1faef --- /dev/null +++ b/backend/src/__tests__/node-management-hardening.test.ts @@ -0,0 +1,231 @@ +/** + * Node-management route hardening. + * + * Gate parity: the write routes under /api/nodes enforce node:manage (admin or + * node-admin). viewer and deployer sessions must be refused, while the read + * route stays open to any authenticated session. This is the backend contract + * the frontend mirrors by showing the node table to everyone but gating the + * Add / Edit / Delete affordances on node:manage. + * + * Tunnel cleanup: deleting a node tears down any live pilot tunnel so the + * bridge (loopback server, ping timer, open streams) is released immediately + * instead of lingering until the agent happens to disconnect. + */ +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import request from 'supertest'; +import jwt from 'jsonwebtoken'; +import { EventEmitter } from 'events'; +import { WebSocket } from 'ws'; +import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './helpers/setupTestDb'; +import { PilotTunnelManager } from '../services/PilotTunnelManager'; +import { MeshProxyTunnelDialer } from '../services/MeshProxyTunnelDialer'; + +let tmpDir: string; +let app: import('express').Express; +let DatabaseService: typeof import('../services/DatabaseService').DatabaseService; + +type ManageRole = 'admin' | 'node-admin'; +type DeniedRole = 'viewer' | 'deployer'; + +function authToken(username: string, role: string, tv: number): string { + return jwt.sign({ username, role, tv }, TEST_JWT_SECRET, { expiresIn: '1m' }); +} + +/** + * Token for a session in the given role. The auth middleware resolves the role + * from the DB, so the user must exist; password_hash is irrelevant because we + * sign the JWT directly rather than logging in. The seeded admin is reused so + * we never trip the seat or last-admin guards. + */ +function tokenForRole(role: ManageRole | DeniedRole): string { + const db = DatabaseService.getInstance(); + const username = role === 'admin' ? TEST_USERNAME : `nm-${role}`; + let user = db.getUserByUsername(username); + if (!user) { + db.addUser({ username, password_hash: 'test-hash', role }); + user = db.getUserByUsername(username)!; + } + return authToken(username, role, user.token_version); +} + +function makeMockTunnelWs(): EventEmitter & { + readyState: number; + bufferedAmount: number; + send: (data: unknown) => void; + ping: () => void; + close: () => void; +} { + const ws = new EventEmitter() as EventEmitter & { + readyState: number; + bufferedAmount: number; + send: (data: unknown) => void; + ping: () => void; + close: () => void; + }; + ws.readyState = WebSocket.OPEN; + ws.bufferedAmount = 0; + ws.send = () => { /* no-op */ }; + ws.ping = () => { /* no-op */ }; + ws.close = () => { ws.readyState = WebSocket.CLOSED; ws.emit('close'); }; + return ws; +} + +function addPilotNode(name: string): number { + return DatabaseService.getInstance().addNode({ + name, + type: 'remote', + mode: 'pilot_agent', + compose_dir: '/tmp/x', + is_default: false, + api_url: '', + api_token: '', + }); +} + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ DatabaseService } = await import('../services/DatabaseService')); + ({ app } = await import('../index')); +}); + +afterAll(() => cleanupTestDb(tmpDir)); + +describe('node-management write routes require node:manage', () => { + it('lets a viewer read the node list (the table stays visible)', async () => { + const res = await request(app) + .get('/api/nodes') + .set('Authorization', `Bearer ${tokenForRole('viewer')}`); + expect(res.status).toBe(200); + expect(Array.isArray(res.body)).toBe(true); + }); + + for (const role of ['viewer', 'deployer'] as const) { + it(`refuses node creation for ${role} (403 PERMISSION_DENIED)`, async () => { + const res = await request(app) + .post('/api/nodes') + .set('Authorization', `Bearer ${tokenForRole(role)}`) + .send({ name: `nm-create-${role}`, type: 'remote', mode: 'pilot_agent' }); + expect(res.status).toBe(403); + expect(res.body.code).toBe('PERMISSION_DENIED'); + }); + + it(`refuses node deletion for ${role} and leaves the node intact (403)`, async () => { + const db = DatabaseService.getInstance(); + const id = addPilotNode(`nm-del-${role}`); + const res = await request(app) + .delete(`/api/nodes/${id}`) + .set('Authorization', `Bearer ${tokenForRole(role)}`); + expect(res.status).toBe(403); + expect(res.body.code).toBe('PERMISSION_DENIED'); + expect(db.getNode(id)).toBeTruthy(); + db.deleteNode(id); + }); + } + + // Positive boundary: both manage-capable roles succeed. admin short-circuits + // the permission engine; node-admin must resolve node:manage from + // ROLE_PERMISSIONS, so this also guards against node-admin silently losing + // write access if that mapping ever changes. + for (const role of ['admin', 'node-admin'] as const) { + it(`allows node creation for ${role} (200)`, async () => { + const res = await request(app) + .post('/api/nodes') + .set('Authorization', `Bearer ${tokenForRole(role)}`) + .send({ name: `nm-create-${role}-${Date.now()}`, type: 'remote', mode: 'pilot_agent' }); + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + }); + } + + it('allows a node-admin to delete a node (200)', async () => { + const db = DatabaseService.getInstance(); + const id = addPilotNode(`nm-del-nodeadmin-${Date.now()}`); + const res = await request(app) + .delete(`/api/nodes/${id}`) + .set('Authorization', `Bearer ${tokenForRole('node-admin')}`); + expect(res.status).toBe(200); + expect(db.getNode(id)).toBeUndefined(); + }); +}); + +describe('deleting a node tears down its tunnel or mesh bridge', () => { + it('closes the active tunnel socket and removes the node row', async () => { + const db = DatabaseService.getInstance(); + const mgr = PilotTunnelManager.getInstance(); + const id = addPilotNode(`nm-tunnel-del-${Date.now()}`); + + const ws = makeMockTunnelWs(); + await mgr.registerTunnel(id, ws as unknown as WebSocket, 'test-1.0.0'); + expect(mgr.hasActiveTunnel(id)).toBe(true); + + const res = await request(app) + .delete(`/api/nodes/${id}`) + .set('Authorization', `Bearer ${tokenForRole('admin')}`); + + expect(res.status).toBe(200); + // The manager forgot the tunnel AND the underlying socket was actually + // closed (not just dropped from the map), so the agent gets a clean close. + expect(mgr.hasActiveTunnel(id)).toBe(false); + expect(ws.readyState).toBe(WebSocket.CLOSED); + expect(db.getNode(id)).toBeUndefined(); + }); + + it('deletes a node with no active tunnel without error (closeTunnel no-op path)', async () => { + const db = DatabaseService.getInstance(); + const id = addPilotNode(`nm-no-tunnel-del-${Date.now()}`); + expect(PilotTunnelManager.getInstance().hasActiveTunnel(id)).toBe(false); + + const res = await request(app) + .delete(`/api/nodes/${id}`) + .set('Authorization', `Bearer ${tokenForRole('admin')}`); + + expect(res.status).toBe(200); + expect(db.getNode(id)).toBeUndefined(); + }); + + it('closes a live proxy-mode mesh bridge on delete without scheduling a redial', async () => { + const db = DatabaseService.getInstance(); + const dialer = MeshProxyTunnelDialer.resetForTest(); + const redialSpy = vi.spyOn( + dialer as unknown as { scheduleReactiveRedial: (id: number) => void }, + 'scheduleReactiveRedial', + ).mockImplementation(() => {}); + + const id = db.addNode({ + name: `nm-proxy-del-${Date.now()}`, + type: 'remote', + mode: 'proxy', + api_url: 'http://proxy-peer:1852', + api_token: 'tok', + compose_dir: '/tmp/x', + is_default: false, + }); + + // Prime a live proxy bridge the way dial() does: in the dialer's map with a + // close listener wired. A real bridge emits 'closed' when closed, so the + // fake does too. If the delete path closed it via the manager instead of the + // dialer's intentional path, that 'closed' event would drive tearDownBridge + // -> proxy-bridge-down -> a reactive redial against the deleted node. + const fakeBridge = new EventEmitter() as EventEmitter & { + close: ReturnType; + getActiveStreamCount: () => number; + }; + fakeBridge.close = vi.fn(() => { fakeBridge.emit('closed', { code: 1000 }); }); + fakeBridge.getActiveStreamCount = () => 0; + (dialer as unknown as { bridges: Map }).bridges.set(id, fakeBridge); + (dialer as unknown as { attachBridgeCloseListener: (id: number, b: EventEmitter) => void }) + .attachBridgeCloseListener(id, fakeBridge); + + const res = await request(app) + .delete(`/api/nodes/${id}`) + .set('Authorization', `Bearer ${tokenForRole('admin')}`); + + expect(res.status).toBe(200); + expect(fakeBridge.close).toHaveBeenCalled(); + expect(dialer.hasBridge(id)).toBe(false); + expect(redialSpy).not.toHaveBeenCalled(); + expect(db.getNode(id)).toBeUndefined(); + + redialSpy.mockRestore(); + }); +}); diff --git a/backend/src/pilot/protocol.ts b/backend/src/pilot/protocol.ts index bb123f16..2759af23 100644 --- a/backend/src/pilot/protocol.ts +++ b/backend/src/pilot/protocol.ts @@ -308,6 +308,7 @@ export function wsDataToString(data: unknown): string | null { // --- Close codes --- export const PilotCloseCode = { + NormalClosure: 1000, Replaced: 4000, EnrollmentRegenerated: 4001, ProtocolError: 1002, diff --git a/backend/src/routes/nodes.ts b/backend/src/routes/nodes.ts index cea22aa3..e8809aa7 100644 --- a/backend/src/routes/nodes.ts +++ b/backend/src/routes/nodes.ts @@ -313,6 +313,15 @@ nodesRouter.delete('/:id', async (req: Request, res: Response) => { if (!requirePermission(req, res, 'node:manage', 'node', nodeIdParam)) return; try { const id = parseInt(nodeIdParam); + // Release any live tunnel or mesh bridge before deleting the record so it is + // freed immediately rather than lingering until the peer disconnects. Close a + // proxy-mode mesh bridge through its dialer FIRST: closeBridge removes the + // bridge before closing it, so the dialer does not schedule a reactive redial + // against a node that is about to disappear. closeTunnel then closes a + // pilot-agent tunnel; both are no-ops when this node has no such bridge (a + // local node, or one with no active connection). Mirrors the re-enroll path. + MeshProxyTunnelDialer.getInstance().closeBridge(id, 'node deleted'); + PilotTunnelManager.getInstance().closeTunnel(id, PilotCloseCode.NormalClosure, 'node deleted'); DatabaseService.getInstance().deleteNode(id); NodeRegistry.getInstance().evictConnection(id); NodeRegistry.getInstance().notifyNodeRemoved(id); diff --git a/frontend/src/components/NodeManager.tsx b/frontend/src/components/NodeManager.tsx index 46cd3c15..441ae5ea 100644 --- a/frontend/src/components/NodeManager.tsx +++ b/frontend/src/components/NodeManager.tsx @@ -35,8 +35,15 @@ export interface SenchoNavigateDetail { export function NodeManager() { const { isPaid } = useLicense(); - const { isAdmin } = useAuth(); + const { isAdmin, can } = useAuth(); const canEditLabels = isPaid && isAdmin; + // Mirror the backend node:manage guard. This top-level flag checks the global + // role only (admin or global node-admin); the per-row Edit/Delete buttons below + // additionally honor scoped per-node grants via can('node:manage', 'node', id). + // Admins resolve immediately via isAdmin; node-admins once /permissions/me lands. + // Generate-token and reset-anchor below stay admin-only to match their stricter + // backend guards (requireAdmin, and requireAdmin + requirePaid). + const canManageNodes = isAdmin || can('node:manage'); const { nodes, refreshNodeMeta } = useNodes(); useMastheadStats([ { label: 'NODES', value: `${nodes.length}` }, @@ -189,21 +196,29 @@ export function NodeManager() { return (
- {/* Actions */} -
- - - Add node - -
+ {/* Actions (node management is admin / node-admin only, mirroring the + node:manage backend guard). The read-only table below stays visible to + every role with node:read. */} + {canManageNodes && ( + <> +
+ + + Add node + +
- + + + )} - {/* Generate Node Token - for use on THIS instance as a remote target */} + {/* Generate a node token so THIS instance can serve as a remote target. + Admin-only, matching the requireAdmin guard on /auth/generate-node-token. */} + {isAdmin && (
@@ -235,6 +250,7 @@ export function NodeManager() {
)}
+ )} {/* Sync issues: surfaces FleetSync sticky errors (currently CONTROL_IDENTITY_MISMATCH). */} {anchorMismatches.length > 0 && ( @@ -257,15 +273,17 @@ export function NodeManager() { {' '}Reset the anchor on the peer to resume sync, or remove the node from this fleet.
- - {node && !node.is_default && ( + {isAdmin && isPaid && ( + + )} + {node && !node.is_default && (isAdmin || can('node:manage', 'node', String(nodeId))) && ( - - Edit Node - - + {canManageThis && ( + + + + + + Edit Node + + + )} - {!node.is_default && ( + {!node.is_default && canManageThis && ( @@ -489,7 +511,8 @@ export function NodeManager() {
- ))} + ); + })}
diff --git a/frontend/src/components/__tests__/NodeManager.test.tsx b/frontend/src/components/__tests__/NodeManager.test.tsx new file mode 100644 index 00000000..9a9965eb --- /dev/null +++ b/frontend/src/components/__tests__/NodeManager.test.tsx @@ -0,0 +1,96 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import type { Node } from '@/context/NodeContext'; + +// NodeManager pulls in several contexts and a child action hook. Mock them so +// the test renders the panel in isolation and can drive the role/permission +// inputs that gate the write affordances. +const useAuthMock = vi.fn(); +const useLicenseMock = vi.fn(); + +const testNode: Node = { + id: 2, + name: 'Edge', + type: 'remote', + mode: 'pilot_agent', + compose_dir: '/app/compose', + is_default: false, + status: 'online', + created_at: 0, + api_url: '', + pilot_last_seen: Date.now(), +}; + +vi.mock('@/context/NodeContext', () => ({ + useNodes: () => ({ nodes: [testNode], refreshNodeMeta: vi.fn() }), +})); +vi.mock('@/context/AuthContext', () => ({ useAuth: () => useAuthMock() })); +vi.mock('@/context/LicenseContext', () => ({ useLicense: () => useLicenseMock() })); +vi.mock('@/lib/api', () => ({ + apiFetch: vi.fn(() => Promise.resolve({ ok: false, json: () => Promise.resolve({}) })), +})); +vi.mock('@/components/ui/toast-store', () => ({ + toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn() }, +})); +vi.mock('@/hooks/useFleetSyncStatus', () => ({ + useFleetSyncStatus: () => ({ statuses: [], refresh: vi.fn() }), +})); +vi.mock('../settings/MastheadStatsContext', () => ({ useMastheadStats: vi.fn() })); +vi.mock('../nodes/useNodeActions', () => ({ + useNodeActions: () => ({ + openCreate: vi.fn(), + openEdit: vi.fn(), + openDelete: vi.fn(), + NodeActionModals: null, + }), +})); +vi.mock('../blueprints/NodeLabelPicker', () => ({ NodeLabelPicker: () => null })); + +import { NodeManager } from '../NodeManager'; + +/** can() that grants only the named action regardless of resource scope. */ +function canFor(...granted: string[]) { + return (action: string) => granted.includes(action); +} + +beforeEach(() => { + useLicenseMock.mockReturnValue({ isPaid: false }); +}); +afterEach(() => vi.clearAllMocks()); + +describe('NodeManager write-affordance gating', () => { + it('hides every write affordance from a viewer but still shows the node table', () => { + useAuthMock.mockReturnValue({ isAdmin: false, can: canFor() }); + render(); + + // Read-only surface stays visible. + expect(screen.getByText('Edge')).toBeInTheDocument(); + + expect(screen.queryByRole('button', { name: /Add node/i })).not.toBeInTheDocument(); + expect(screen.queryByText('Generate Node Token')).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Edit node' })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Delete node' })).not.toBeInTheDocument(); + }); + + it('shows every affordance to an admin', () => { + useAuthMock.mockReturnValue({ isAdmin: true, can: canFor('node:manage') }); + render(); + + expect(screen.getByRole('button', { name: /Add node/i })).toBeInTheDocument(); + expect(screen.getByText('Generate Node Token')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Edit node' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Delete node' })).toBeInTheDocument(); + }); + + it('lets a node-admin manage nodes but hides the admin-only token card', () => { + // node-admin: holds node:manage but is not a global admin. + useAuthMock.mockReturnValue({ isAdmin: false, can: canFor('node:manage') }); + render(); + + expect(screen.getByRole('button', { name: /Add node/i })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Edit node' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Delete node' })).toBeInTheDocument(); + // Generate Node Token mirrors requireAdmin, so a node-admin must not see it. + expect(screen.queryByText('Generate Node Token')).not.toBeInTheDocument(); + }); +});