diff --git a/backend/src/__tests__/fleet-action-card-endpoints.test.ts b/backend/src/__tests__/fleet-action-card-endpoints.test.ts index 01eb8313..f622cdbd 100644 --- a/backend/src/__tests__/fleet-action-card-endpoints.test.ts +++ b/backend/src/__tests__/fleet-action-card-endpoints.test.ts @@ -22,6 +22,11 @@ const getContainersByStack = vi.fn(); const stopContainer = vi.fn(); const restartContainer = vi.fn(); const invalidateNodeCaches = vi.fn(); +const remoteSupportsCrossNodeRbac = vi.fn(); + +vi.mock('../helpers/remoteCapabilities', () => ({ + remoteSupportsCrossNodeRbac, +})); vi.mock('../services/FileSystemService', () => ({ FileSystemService: { @@ -83,6 +88,9 @@ beforeEach(() => { getContainersByStack.mockResolvedValue([{ Id: 'container-1' }]); stopContainer.mockResolvedValue(undefined); restartContainer.mockResolvedValue(undefined); + // Default: remotes are upgraded and honor the exact-stack allowlist. The + // mixed-version cases below flip this to false to exercise the gate. + remoteSupportsCrossNodeRbac.mockResolvedValue(true); activeBulkActions.clear(); db.getDb().prepare('DELETE FROM stack_label_assignments').run(); db.getDb().prepare('DELETE FROM stack_labels').run(); @@ -845,6 +853,60 @@ describe('POST /api/fleet/labels/fleet-stop confirmed-target allowlist', () => { expect(res.status).toBe(400); expect(res.body.error).toMatch(/nodeId/); }); + + it('refuses a real stop to a remote lacking cross-node-rbac and reports it as needing upgrade', async () => { + remoteSupportsCrossNodeRbac.mockResolvedValue(false); + const remoteId = db.addNode({ + name: 'old-remote', type: 'remote', api_url: 'http://old.example:1852', + api_token: 'tok', compose_dir: '/app/compose', is_default: false, + }); + try { + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: 'any-label', targets: [{ nodeId: remoteId, stackNames: ['alpha'] }] }); + expect(res.status).toBe(200); + const node = res.body.results.find((r: { nodeId: number }) => r.nodeId === remoteId); + expect(node.reachable).toBe(false); + expect(node.error).toMatch(/upgrade/i); + expect(node.stackResults).toEqual([{ stackName: 'alpha', success: false, error: 'Node must be upgraded to honor an exact-stack stop' }]); + // The destructive local-stop receiver is never contacted on an old remote. + expect(fetchSpy.mock.calls.some(c => String(c[0]).endsWith('/api/fleet-actions/labels/local-stop'))).toBe(false); + } finally { + db.deleteNode(remoteId); + } + }); + + it('fails a node whose stop results name stacks outside the confirmed set', async () => { + remoteSupportsCrossNodeRbac.mockResolvedValue(true); + const remoteId = db.addNode({ + name: 'lying-remote', type: 'remote', api_url: 'http://lying.example:1852', + api_token: 'tok', compose_dir: '/app/compose', is_default: false, + }); + try { + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, status: 200, + json: async () => ({ + matched: true, + results: [ + { stackName: 'alpha', success: true }, + { stackName: 'unconfirmed-extra', success: true }, + ], + }), + } as unknown as Response); + const res = await request(app) + .post('/api/fleet/labels/fleet-stop') + .set('Authorization', authHeader) + .send({ labelName: 'any-label', targets: [{ nodeId: remoteId, stackNames: ['alpha'] }] }); + expect(res.status).toBe(200); + const node = res.body.results.find((r: { nodeId: number }) => r.nodeId === remoteId); + expect(node.reachable).toBe(false); + expect(node.error).toMatch(/outside the confirmed set/i); + } finally { + db.deleteNode(remoteId); + } + }); }); describe('POST /api/fleet/labels/fleet-stop with dryRun: true', () => { diff --git a/backend/src/__tests__/fleet-actions.test.ts b/backend/src/__tests__/fleet-actions.test.ts index b4e058d0..d7261865 100644 --- a/backend/src/__tests__/fleet-actions.test.ts +++ b/backend/src/__tests__/fleet-actions.test.ts @@ -321,6 +321,30 @@ describe('local-stop behavior', () => { } }); + it('reports every confirmed stack under lock contention, including one that lost its label', async () => { + const label = db.createLabel(nodeId, 'contend-label', 'slate'); + db.setStackLabels('contend-kept', nodeId, [label.id]); + // contend-lost was confirmed in the preview but no longer carries the label. + const { activeBulkActions } = await import('../routes/labels'); + activeBulkActions.add(`bulk:${nodeId}`); + try { + const res = await request(app) + .post('/api/fleet-actions/labels/local-stop') + .set('Authorization', authHeader) + .send({ labelName: 'contend-label', stackNames: ['contend-kept', 'contend-lost'] }); + expect(res.status).toBe(200); + // Both confirmed stacks surface as contention failures; the lost one is + // not dropped (which would read as "no stacks assigned"), and the result + // is never empty when stacks were confirmed. + expect(res.body.results).toHaveLength(2); + const byName = Object.fromEntries(res.body.results.map((r: { stackName: string }) => [r.stackName, r])); + expect(byName['contend-kept'].error).toMatch(/already running/); + expect(byName['contend-lost'].error).toMatch(/already running/); + } finally { + activeBulkActions.delete(`bulk:${nodeId}`); + } + }); + it('dry run returns dryRun:true per on-disk stack without touching Docker', async () => { makeStack('dry-stack'); const label = db.createLabel(nodeId, 'dry-label', 'slate'); diff --git a/backend/src/__tests__/proxy-cross-node-rbac-gate.test.ts b/backend/src/__tests__/proxy-cross-node-rbac-gate.test.ts new file mode 100644 index 00000000..10e5e4f8 --- /dev/null +++ b/backend/src/__tests__/proxy-cross-node-rbac-gate.test.ts @@ -0,0 +1,102 @@ +/** + * Mixed-version HTTP proxy gate. A remote that does not advertise the + * cross-node-rbac capability ignores the forwarded actor role and runs a + * proxied request as admin, so the hub must refuse to proxy a non-admin user's + * request to such a remote. Admins are unaffected. Each remote is a small + * loopback server that answers /api/meta with or without the capability. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import http from 'http'; +import bcrypt from 'bcrypt'; +import request from 'supertest'; +import jwt from 'jsonwebtoken'; +import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './helpers/setupTestDb'; + +let tmpDir: string; +let app: import('express').Express; +let viewerBearer: string; +let adminBearer: string; +let capServer: http.Server; +let noCapServer: http.Server; +let capNodeId: number; +let noCapNodeId: number; + +const PROXIED_PATH = '/api/stacks'; + +function metaServer(capabilities: string[]): http.Server { + return http.createServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + if (req.url?.startsWith('/api/meta')) { + res.end(JSON.stringify({ version: '0.93.0', capabilities })); + } else { + // Any proxied request that clears the gate lands here. + res.end('[]'); + } + }); +} + +async function listen(server: http.Server): Promise { + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + return (server.address() as import('net').AddressInfo).port; +} + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ app } = await import('../index')); + const { DatabaseService } = await import('../services/DatabaseService'); + const db = DatabaseService.getInstance(); + + const hash = await bcrypt.hash('password123', 1); + db.addUser({ username: 'gate-viewer', password_hash: hash, role: 'viewer' }); + const viewer = db.getUserByUsername('gate-viewer')!; + viewerBearer = jwt.sign({ username: 'gate-viewer', role: 'viewer', tv: viewer.token_version }, TEST_JWT_SECRET, { expiresIn: '1m' }); + adminBearer = jwt.sign({ username: TEST_USERNAME }, TEST_JWT_SECRET, { expiresIn: '1m' }); + + capServer = metaServer(['cross-node-rbac']); + noCapServer = metaServer(['fleet']); // older remote: no cross-node-rbac + const capPort = await listen(capServer); + const noCapPort = await listen(noCapServer); + + capNodeId = db.addNode({ + name: 'cap-remote', type: 'remote', mode: 'proxy', compose_dir: '/tmp', + is_default: false, api_url: `http://127.0.0.1:${capPort}`, api_token: 'cap-token', + }); + noCapNodeId = db.addNode({ + name: 'nocap-remote', type: 'remote', mode: 'proxy', compose_dir: '/tmp', + is_default: false, api_url: `http://127.0.0.1:${noCapPort}`, api_token: 'nocap-token', + }); +}); + +afterAll(async () => { + await new Promise((resolve) => capServer.close(() => resolve())); + await new Promise((resolve) => noCapServer.close(() => resolve())); + cleanupTestDb(tmpDir); +}); + +describe('remote proxy cross-node-rbac gate', () => { + it('denies a non-admin proxied request to a remote that lacks cross-node-rbac', async () => { + const res = await request(app) + .get(PROXIED_PATH) + .set('Authorization', `Bearer ${viewerBearer}`) + .set('x-node-id', String(noCapNodeId)); + expect(res.status).toBe(403); + expect(res.body.error).toMatch(/upgrade/i); + }); + + it('allows a non-admin proxied request to a remote that advertises cross-node-rbac', async () => { + const res = await request(app) + .get(PROXIED_PATH) + .set('Authorization', `Bearer ${viewerBearer}`) + .set('x-node-id', String(capNodeId)); + // Forwarded to the capability-advertising remote, which answers 200. + expect(res.status).not.toBe(403); + }); + + it('does not gate an admin proxied request even to a remote lacking cross-node-rbac', async () => { + const res = await request(app) + .get(PROXIED_PATH) + .set('Authorization', `Bearer ${adminBearer}`) + .set('x-node-id', String(noCapNodeId)); + expect(res.status).not.toBe(403); + }); +}); diff --git a/backend/src/__tests__/proxy-role-forwarding.test.ts b/backend/src/__tests__/proxy-role-forwarding.test.ts index b4c18b00..98257fd7 100644 --- a/backend/src/__tests__/proxy-role-forwarding.test.ts +++ b/backend/src/__tests__/proxy-role-forwarding.test.ts @@ -171,6 +171,14 @@ describe('remote proxy gateway - actor role header forwarding', () => { beforeAll(async () => { captureServer = http.createServer((req, res) => { + // The proxy gate probes /api/meta first to confirm the remote enforces + // cross-node RBAC before forwarding a non-admin request; advertise it so + // the viewer forward is allowed and we can assert the forwarded headers. + if (req.url?.startsWith('/api/meta')) { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ version: '0.93.0', capabilities: ['cross-node-rbac'] })); + return; + } captured = req.headers; res.writeHead(200, { 'Content-Type': 'application/json' }); res.end('[]'); diff --git a/backend/src/__tests__/remote-capabilities.test.ts b/backend/src/__tests__/remote-capabilities.test.ts new file mode 100644 index 00000000..9868fc78 --- /dev/null +++ b/backend/src/__tests__/remote-capabilities.test.ts @@ -0,0 +1,58 @@ +/** + * Unit coverage for the cross-node-rbac capability probe used to gate + * mixed-version cross-node operations. It reads the shared remote-meta cache + * (fetching once on a cold miss) and must fail closed: a node whose capability + * cannot be determined is treated as unsupported. + */ +import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest'; +import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb'; +import type { RemoteMeta } from '../services/CapabilityRegistry'; + +let remoteSupportsCrossNodeRbac: typeof import('../helpers/remoteCapabilities').remoteSupportsCrossNodeRbac; +let NodeRegistry: typeof import('../services/NodeRegistry').NodeRegistry; +let CacheService: typeof import('../services/CacheService').CacheService; +let tmpDir: string; + +const NODE_ID = 4242; + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ remoteSupportsCrossNodeRbac } = await import('../helpers/remoteCapabilities')); + ({ NodeRegistry } = await import('../services/NodeRegistry')); + ({ CacheService } = await import('../services/CacheService')); +}); + +afterAll(() => cleanupTestDb(tmpDir)); + +afterEach(() => { + vi.restoreAllMocks(); + CacheService.getInstance().invalidate(`remote-meta:${NODE_ID}`); +}); + +function mockMeta(meta: RemoteMeta): void { + vi.spyOn(NodeRegistry.getInstance(), 'fetchMetaForNode').mockResolvedValue(meta); +} + +const ONLINE = { startedAt: null, updateError: null, online: true } as const; + +describe('remoteSupportsCrossNodeRbac', () => { + it('returns true when the remote advertises cross-node-rbac', async () => { + mockMeta({ version: '0.93.0', capabilities: ['fleet', 'cross-node-rbac'], ...ONLINE }); + expect(await remoteSupportsCrossNodeRbac(NODE_ID)).toBe(true); + }); + + it('returns false when the remote does not advertise it', async () => { + mockMeta({ version: '0.92.0', capabilities: ['fleet', 'labels'], ...ONLINE }); + expect(await remoteSupportsCrossNodeRbac(NODE_ID)).toBe(false); + }); + + it('fails closed when the remote meta has no resolvable version', async () => { + mockMeta({ version: null, capabilities: [], startedAt: null, updateError: null, online: false }); + expect(await remoteSupportsCrossNodeRbac(NODE_ID)).toBe(false); + }); + + it('fails closed when the meta fetch throws (unreachable)', async () => { + vi.spyOn(NodeRegistry.getInstance(), 'fetchMetaForNode').mockRejectedValue(new Error('unreachable')); + expect(await remoteSupportsCrossNodeRbac(NODE_ID)).toBe(false); + }); +}); diff --git a/backend/src/helpers/fleetLabelStop.ts b/backend/src/helpers/fleetLabelStop.ts index 657ae427..14a83378 100644 --- a/backend/src/helpers/fleetLabelStop.ts +++ b/backend/src/helpers/fleetLabelStop.ts @@ -88,7 +88,12 @@ export async function runLocalLabelStop( const lockKey = `bulk:${nodeId}`; if (activeBulkActions.has(lockKey)) { - const busyStacks = allowedStacks ? stackNames.filter(name => allowedStacks.has(name)) : stackNames; + // Report every confirmed stack as a contention failure, not just the + // currently label-matched subset: a confirmed stack that lost its label + // must still surface (the operator asked to stop it), and the result must + // not be empty when stacks were confirmed, or the UI reads it as "label + // matched but no stacks assigned" instead of a contention failure. + const busyStacks = allowedStacks ? [...allowedStacks] : stackNames; return { matched: true, stackResults: busyStacks.map(stackName => ({ diff --git a/backend/src/helpers/remoteCapabilities.ts b/backend/src/helpers/remoteCapabilities.ts new file mode 100644 index 00000000..5076939e --- /dev/null +++ b/backend/src/helpers/remoteCapabilities.ts @@ -0,0 +1,44 @@ +import { CacheService } from '../services/CacheService'; +import { NodeRegistry } from '../services/NodeRegistry'; +import { CROSS_NODE_RBAC_CAPABILITY, type RemoteMeta } from '../services/CapabilityRegistry'; +import { REMOTE_META_NAMESPACE } from './cacheInvalidation'; +import { getErrorMessage } from '../utils/errors'; + +// Mirrors the node-meta endpoint's TTL and shares its `remote-meta:` cache +// key, so a recent /api/nodes/:id/meta read warms this check and vice versa. +const REMOTE_META_CACHE_TTL = 3 * 60 * 1000; + +/** + * Whether a remote node advertises that it enforces cross-node RBAC: the + * forwarded actor role on HTTP requests and the exact-stack allowlist on + * stop-by-label. Reads the shared remote-meta cache, fetching once on a cold + * miss. + * + * Fails closed: when the capability cannot be established (an un-upgraded + * remote, or a cold cache whose meta fetch fails) it returns false, so the + * caller denies rather than risk escalating a non-admin request or over-stopping + * on an un-upgraded node. A node previously cached as supported may be served + * that value while a later refresh is failing (getOrFetch serves stale on + * error); that is safe because capabilities are append-only and a request to an + * unreachable node fails at the transport regardless. + */ +export async function remoteSupportsCrossNodeRbac(nodeId: number): Promise { + try { + const meta = await CacheService.getInstance().getOrFetch( + `${REMOTE_META_NAMESPACE}:${nodeId}`, + REMOTE_META_CACHE_TTL, + async () => { + const fetched = await NodeRegistry.getInstance().fetchMetaForNode(nodeId); + if (fetched.version === null) throw new Error('Remote meta fetch returned null version'); + return fetched; + }, + ); + return meta.capabilities.includes(CROSS_NODE_RBAC_CAPABILITY); + } catch (err) { + console.warn( + `[CrossNodeRBAC] Could not determine capability for node ${nodeId}; treating as unsupported:`, + getErrorMessage(err, 'unknown'), + ); + return false; + } +} diff --git a/backend/src/proxy/remoteNodeProxy.ts b/backend/src/proxy/remoteNodeProxy.ts index 9147c4f2..d2a1b56b 100644 --- a/backend/src/proxy/remoteNodeProxy.ts +++ b/backend/src/proxy/remoteNodeProxy.ts @@ -4,6 +4,7 @@ import { NodeRegistry } from '../services/NodeRegistry'; import { PROXY_TIER_HEADER, PROXY_ROLE_HEADER } from '../services/license-headers'; import { LicenseService } from '../services/LicenseService'; import { isProxyExemptPath } from '../helpers/proxyExemptPaths'; +import { remoteSupportsCrossNodeRbac } from '../helpers/remoteCapabilities'; import { getErrorMessage } from '../utils/errors'; import { DatabaseService } from '../services/DatabaseService'; import { redactSensitiveText } from '../utils/safeLog'; @@ -145,6 +146,30 @@ export function createRemoteProxyMiddleware(): RequestHandler { return; } + // Mixed-version RBAC gate. The forwarded actor role is enforced only by a + // remote that advertises cross-node-rbac; an older remote ignores the + // header and runs the proxied request as admin. So a non-admin must not be + // forwarded to a remote that does not advertise the capability. Admins are + // unaffected (they are admin on the remote regardless), and the check is + // skipped for them so it never adds latency to the admin path. Fails closed + // when the capability cannot be determined. Using `?.` so an unresolved user + // (not reachable past authGate, but defensive) is gated, never waved through. + if (req.user?.role !== 'admin') { + remoteSupportsCrossNodeRbac(req.nodeId) + .then((supported) => { + if (!supported) { + res.status(403).json({ + error: `Remote node "${node.name}" is running a version that does not enforce per-user permissions. Upgrade it before non-admin users can act on it.`, + }); + return; + } + req.proxyTarget = target; + proxy(req, res, next); + }) + .catch(next); + return; + } + req.proxyTarget = target; proxy(req, res, next); }; diff --git a/backend/src/routes/fleet.ts b/backend/src/routes/fleet.ts index fa791639..f6bc223c 100644 --- a/backend/src/routes/fleet.ts +++ b/backend/src/routes/fleet.ts @@ -20,6 +20,7 @@ import { requirePaid, requireAdmin, requireNodeProxy } from '../middleware/tierG import { requirePermission } from '../middleware/permissions'; import { scheduleLocalUpdate } from './license'; import { runPolicyGate, assertPolicyGateAllows, buildPolicyGateOptions } from '../helpers/policyGate'; +import { remoteSupportsCrossNodeRbac } from '../helpers/remoteCapabilities'; import { captureLocalNodeFiles, captureRemoteNodeFiles, buildSnapshotDocumentation, pickDossierFields, dossierHasContent, type SnapshotNodeData, type SnapshotDocumentation } from '../utils/snapshot-capture'; import { getLatestVersion, getLatestRelease } from '../utils/version-check'; import { isValidStackName } from '../utils/validation'; @@ -1464,6 +1465,22 @@ fleetRouter.post('/labels/fleet-stop', authMiddleware, async (req: Request, res: if (!target) { return { nodeId: node.id, nodeName: node.name, reachable: false, matched: false, stackResults: [], error: formatNoTargetError(node) }; } + // A real stop bound to a confirmed stack set must only go to a remote that + // honors the allowlist (cross-node-rbac). An older remote ignores + // `stackNames` and would stop every label-matched stack, including ones + // labelled after the preview. Refuse to send and report the node as + // needing an upgrade instead. Dry runs carry no allowlist and only + // preview, so they are safe to send to any version. + if (!isDryRun && allowedStacks) { + const supported = await remoteSupportsCrossNodeRbac(node.id); + if (!supported) { + return { + nodeId: node.id, nodeName: node.name, reachable: false, matched: false, + stackResults: failAllStacks([...allowedStacks], 'Node must be upgraded to honor an exact-stack stop'), + error: 'Node is running a version that cannot limit the stop to the confirmed stacks; upgrade it and retry.', + }; + } + } try { const headers: Record = { 'Content-Type': 'application/json' }; if (target.apiToken) headers.Authorization = `Bearer ${target.apiToken}`; @@ -1490,6 +1507,14 @@ fleetRouter.post('/labels/fleet-stop', authMiddleware, async (req: Request, res: if (!isLabelLocalStopResponse(remote)) { return { nodeId: node.id, nodeName: node.name, reachable: false, matched: false, stackResults: [], error: 'Remote returned a malformed response' }; } + // Defense in depth behind the capability gate: if a confirmed allowlist + // was sent, the remote must report only stacks from it. A result naming + // a stack outside the confirmed set means the remote ignored the + // allowlist (an over-broad stop), so fail the node rather than render + // the extra stops as a clean result. + if (allowedStacks && !remote.results.every(r => allowedStacks.has(r.stackName))) { + return { nodeId: node.id, nodeName: node.name, reachable: false, matched: false, stackResults: [], error: 'Remote stopped stacks outside the confirmed set' }; + } return { nodeId: node.id, nodeName: node.name, reachable: true, matched: remote.matched, diff --git a/backend/src/services/CapabilityRegistry.ts b/backend/src/services/CapabilityRegistry.ts index 393b9db3..2d5cda75 100644 --- a/backend/src/services/CapabilityRegistry.ts +++ b/backend/src/services/CapabilityRegistry.ts @@ -41,8 +41,18 @@ export const CAPABILITIES = [ 'env-inventory', 'project-env-files', 'compose-storage', + 'cross-node-rbac', ] as const; +/** + * Advertised by instances that enforce the proxied actor's role (instead of + * treating every node-to-node request as admin) and honor the exact-stack + * allowlist on stop-by-label. The control instance refuses to forward a + * non-admin's request, or a confirmed stop, to a remote lacking this flag so a + * mixed-version fleet cannot escalate or over-stop on an un-upgraded node. + */ +export const CROSS_NODE_RBAC_CAPABILITY = 'cross-node-rbac'; + export type Capability = (typeof CAPABILITIES)[number]; /** Returns true when the string is a usable semver version. */ diff --git a/frontend/src/lib/capabilities.ts b/frontend/src/lib/capabilities.ts index 8f1a0a44..65b00ec0 100644 --- a/frontend/src/lib/capabilities.ts +++ b/frontend/src/lib/capabilities.ts @@ -29,6 +29,7 @@ export const CAPABILITIES = [ 'env-inventory', 'project-env-files', 'compose-storage', + 'cross-node-rbac', ] as const; export type Capability = (typeof CAPABILITIES)[number];