fix: forward scoped node-admin permission for Settings writes through remote proxy (#1748)

* fix: forward scoped node-admin permission for Settings writes through remote proxy The proxy forwards only the user's global role via PROXY_ROLE_HEADER to remote nodes. Scoped role assignments live only on the hub's role_assignments table and are never transmitted, so a scoped Node Admin could not save settings on their granted remote node through the hub proxy. Add a settings-write pre-authorization gate in runGatedProxy that buffers the body, extracts required permission buckets from SETTING_WRITE_PERMISSIONS, checks them hub-side, and elevates PROXY_ROLE_HEADER to 'node-admin' when the scoped check passes. The gate is fail-closed: empty or unparseable bodies require checkNodeManage on the hub, matching the existing requireSettingsWritePermission empty-keys branch. Fixes the gate-parity gap where scoped node:manage worked locally but not through the proxy for Settings writes. * fix: remove unused UserRole import from remoteNodeProxy.ts * test(self-update): poll instead of a fixed delay in triggerUpdate assertion The 600ms sleep raced the route's 500ms post-response timer plus the persist/watch work executeClaimedCommunityUpdate does before calling triggerUpdate, leaving too little margin under CI's forked test pool. Poll with vi.waitFor instead, matching the pattern already used elsewhere in this suite.
2026-08-12 19:57:37 +00:00 · 2026-08-02 03:00:09 -04:00
parent 7c02f6eeb5
commit ba017ee665
5 changed files with 454 additions and 23 deletions
@@ -0,0 +1,343 @@
+/**
+ * Hub → remote proxy coverage for scoped node-admin Settings writes.
+ * Exercises the settings pre-authorization gate in createRemoteProxyMiddleware
+ * through live loopback remotes.
+ */
+import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
+import http from 'http';
+import bcrypt from 'bcrypt';
+import request from 'supertest';
+import jwt from 'jsonwebtoken';
+import { setupTestDb, cleanupTestDb, TEST_JWT_SECRET } from './helpers/setupTestDb';
+import { PROXY_ROLE_HEADER } from '../services/license-headers';
+
+let tmpDir: string;
+let app: import('express').Express;
+let viewerBearer: string;
+let viewerId: number;
+
+let grantedServer: http.Server;
+let ungrantedServer: http.Server;
+let grantedNodeId: number;
+let ungrantedNodeId: number;
+
+interface CapturedHop {
+  method: string;
+  url: string;
+  roleHeader: string | undefined;
+}
+const grantedHops: CapturedHop[] = [];
+const ungrantedHops: CapturedHop[] = [];
+
+function captureHop(req: http.IncomingMessage, into: CapturedHop[]): void {
+  into.push({
+    method: req.method ?? '',
+    url: req.url ?? '',
+    roleHeader: req.headers[PROXY_ROLE_HEADER] as string | undefined,
+  });
+}
+
+function grantedRemote(): http.Server {
+  return http.createServer((req, res) => {
+    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;
+    }
+    captureHop(req, grantedHops);
+    res.writeHead(200, { 'Content-Type': 'application/json' });
+    res.end(JSON.stringify({ success: true }));
+  });
+}
+
+function ungrantedRemote(): http.Server {
+  return http.createServer((req, res) => {
+    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;
+    }
+    captureHop(req, ungrantedHops);
+    res.writeHead(200, { 'Content-Type': 'application/json' });
+    res.end(JSON.stringify({ success: true }));
+  });
+}
+
+async function listen(server: http.Server): Promise<number> {
+  await new Promise<void>((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 { LicenseService } = await import('../services/LicenseService');
+  vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('paid');
+
+  const db = DatabaseService.getInstance();
+  const hash = await bcrypt.hash('password123', 1);
+  viewerId = db.addUser({
+    username: 'settings-scoped-viewer',
+    password_hash: hash,
+    role: 'viewer',
+  });
+  const viewer = db.getUserByUsername('settings-scoped-viewer')!;
+  viewerBearer = jwt.sign(
+    { username: 'settings-scoped-viewer', role: 'viewer', tv: viewer.token_version },
+    TEST_JWT_SECRET,
+    { expiresIn: '5m' },
+  );
+
+  grantedServer = grantedRemote();
+  ungrantedServer = ungrantedRemote();
+  const grantedPort = await listen(grantedServer);
+  const ungrantedPort = await listen(ungrantedServer);
+
+  grantedNodeId = db.addNode({
+    name: 'settings-granted-remote',
+    type: 'remote',
+    mode: 'proxy',
+    compose_dir: '/tmp',
+    is_default: false,
+    api_url: `http://127.0.0.1:${grantedPort}`,
+    api_token: 'granted-token',
+  });
+  ungrantedNodeId = db.addNode({
+    name: 'settings-ungranted-remote',
+    type: 'remote',
+    mode: 'proxy',
+    compose_dir: '/tmp',
+    is_default: false,
+    api_url: `http://127.0.0.1:${ungrantedPort}`,
+    api_token: 'ungranted-token',
+  });
+});
+
+afterAll(async () => {
+  await new Promise<void>((resolve) => grantedServer.close(() => resolve()));
+  await new Promise<void>((resolve) => ungrantedServer.close(() => resolve()));
+  cleanupTestDb(tmpDir);
+});
+
+beforeEach(async () => {
+  vi.restoreAllMocks();
+  const { LicenseService } = await import('../services/LicenseService');
+  vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('paid');
+  grantedHops.length = 0;
+  ungrantedHops.length = 0;
+});
+
+describe('remote proxy scoped node-admin settings writes', () => {
+  it('allows scoped node-admin on granted remote node and elevates role header', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    db.addRoleAssignment({
+      user_id: viewerId,
+      role: 'node-admin',
+      resource_type: 'node',
+      resource_id: String(grantedNodeId),
+    });
+
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .send({ host_cpu_limit: 85 });
+
+    expect(res.status).toBe(200);
+    const hop = grantedHops.find((h) => h.url?.includes('/settings'));
+    expect(hop).toBeDefined();
+    expect(hop!.roleHeader).toBe('node-admin');
+
+    db.deleteRoleAssignmentsByUser(viewerId);
+  });
+
+  it('denies scoped node-admin on ungranted remote node', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    db.addRoleAssignment({
+      user_id: viewerId,
+      role: 'node-admin',
+      resource_type: 'node',
+      resource_id: String(grantedNodeId),
+    });
+
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(ungrantedNodeId))
+      .send({ host_cpu_limit: 85 });
+
+    expect(res.status).toBe(403);
+    expect(res.body.code).toBe('PERMISSION_DENIED');
+
+    db.deleteRoleAssignmentsByUser(viewerId);
+  });
+
+  it('denies viewer with no scoped grant on any remote node', async () => {
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .send({ host_cpu_limit: 85 });
+
+    expect(res.status).toBe(403);
+    expect(res.body.code).toBe('PERMISSION_DENIED');
+  });
+
+  it('denies empty-body PATCH from viewer with no grant (fail-closed)', async () => {
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .send({});
+
+    expect(res.status).toBe(403);
+    expect(res.body.code).toBe('PERMISSION_DENIED');
+  });
+
+  it('allows empty-body PATCH from scoped node-admin on granted node', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    db.addRoleAssignment({
+      user_id: viewerId,
+      role: 'node-admin',
+      resource_type: 'node',
+      resource_id: String(grantedNodeId),
+    });
+
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .send({});
+
+    expect(res.status).toBe(200);
+    const hop = grantedHops.find((h) => h.url?.includes('/settings'));
+    expect(hop).toBeDefined();
+    expect(hop!.roleHeader).toBe('node-admin');
+
+    db.deleteRoleAssignmentsByUser(viewerId);
+  });
+
+  it('denies mixed node:manage + system:settings PATCH from scoped node-admin', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    db.addRoleAssignment({
+      user_id: viewerId,
+      role: 'node-admin',
+      resource_type: 'node',
+      resource_id: String(grantedNodeId),
+    });
+
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .send({ host_cpu_limit: 85, developer_mode: '1' });
+
+    expect(res.status).toBe(403);
+    expect(res.body.code).toBe('PERMISSION_DENIED');
+
+    db.deleteRoleAssignmentsByUser(viewerId);
+  });
+
+  it('rejects compressed settings body with 415', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    db.addRoleAssignment({
+      user_id: viewerId,
+      role: 'node-admin',
+      resource_type: 'node',
+      resource_id: String(grantedNodeId),
+    });
+
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .set('Content-Encoding', 'gzip')
+      .send(Buffer.from('compressed'));
+
+    expect(res.status).toBe(415);
+    expect(res.body.code).toBe('encoding_unsupported');
+
+    db.deleteRoleAssignmentsByUser(viewerId);
+  });
+
+  it('rejects oversized settings body with 413', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    db.addRoleAssignment({
+      user_id: viewerId,
+      role: 'node-admin',
+      resource_type: 'node',
+      resource_id: String(grantedNodeId),
+    });
+
+    // Build a body larger than 100 KB
+    const bigValue = 'x'.repeat(102 * 1024);
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .set('Content-Length', String(bigValue.length + 30))
+      .send(Buffer.from(bigValue));
+
+    expect(res.status).toBe(413);
+    expect(res.body.code).toBe('entity_too_large');
+
+    db.deleteRoleAssignmentsByUser(viewerId);
+  });
+
+  it('allows global node-admin to write on remote without elevation gate', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    // The viewer already has role viewer; create a global node-admin
+    const nodeAdminHash = await bcrypt.hash('nodeadmin123', 1);
+    const nodeAdminId = db.addUser({
+      username: 'settings-global-na',
+      password_hash: nodeAdminHash,
+      role: 'node-admin',
+    });
+    const nodeAdmin = db.getUserByUsername('settings-global-na')!;
+    const nodeAdminBearer = jwt.sign(
+      { username: 'settings-global-na', role: 'node-admin', tv: nodeAdmin.token_version },
+      TEST_JWT_SECRET,
+      { expiresIn: '5m' },
+    );
+
+    const res = await request(app)
+      .patch('/api/settings')
+      .set('Authorization', `Bearer ${nodeAdminBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .send({ host_cpu_limit: 90 });
+
+    expect(res.status).toBe(200);
+
+    db.deleteUser(nodeAdminId);
+  });
+
+  it('allows scoped node-admin POST single key on granted node', async () => {
+    const db = (await import('../services/DatabaseService')).DatabaseService.getInstance();
+    db.addRoleAssignment({
+      user_id: viewerId,
+      role: 'node-admin',
+      resource_type: 'node',
+      resource_id: String(grantedNodeId),
+    });
+
+    const res = await request(app)
+      .post('/api/settings')
+      .set('Authorization', `Bearer ${viewerBearer}`)
+      .set('x-node-id', String(grantedNodeId))
+      .send({ key: 'host_cpu_limit', value: 95 });
+
+    expect(res.status).toBe(200);
+    const hop = grantedHops.find((h) => h.url?.includes('/settings'));
+    expect(hop).toBeDefined();
+    expect(hop!.roleHeader).toBe('node-admin');
+
+    db.deleteRoleAssignmentsByUser(viewerId);
+  });
+});
@@ -115,13 +115,14 @@ describe('POST /api/system/update', () => {

    expect(res.status).toBe(202);
    expect(res.body?.message).toMatch(/restart/i);
-    // triggerUpdate runs on res finish + delay; flush the microtask queue.
-    await new Promise(r => setTimeout(r, 600));
-    expect(triggerSpy).toHaveBeenCalledWith(expect.objectContaining({
-      targetVersion: '0.99.0',
-      successMarkerFile: expect.stringMatching(/image-op-success-[\w-]+\.json$/),
-      successMarkerContent: expect.stringMatching(/"operationId":"[\w-]+"/),
-    }));
+    // The route schedules triggerUpdate 500ms after responding, so poll for it.
+    await vi.waitFor(() => {
+      expect(triggerSpy).toHaveBeenCalledWith(expect.objectContaining({
+        targetVersion: '0.99.0',
+        successMarkerFile: expect.stringMatching(/image-op-success-[\w-]+\.json$/),
+        successMarkerContent: expect.stringMatching(/"operationId":"[\w-]+"/),
+      }));
+    }, { timeout: 5_000 });
  });
 });