fix(observability): gate global logs to admins, scope to managed containers, harden SSE (#1254)

* fix(observability): gate global logs to admins, scope to managed containers, harden SSE

Make the Logs feed an administrator view enforced on both sides (requireAdmin on
the /api/logs/global poll and SSE routes; the Logs nav item plus a redirect guard
on the frontend), and scope the feed to Sencho-managed containers only via a
shared isManagedByComposeDir helper that /stats now reuses.

Harden the SSE stream: a stateful frame demuxer that survives chunk boundaries so
a Docker frame split across reads is reassembled instead of dropped or garbled; a
per-stream error listener so one broken follow stream cannot crash the event loop
(it posts a single degraded notice and keeps the others alive); a cap on
concurrent follow streams with a truncation notice; a bounded initial tail; and
backpressure that pauses the source streams when the client is slow and resumes on
drain. Bound the polling snapshot's per-container fan-out with a concurrency limit.

Add process-local, in-memory log-stream counters exposed at the admin-only
/api/system/log-stream-metrics endpoint (active connections, lines streamed,
attach and frame errors). Collapse the view to the local hub and remove the dead
remote-node handling.

* fix(observability): close remote-proxy bypass of the global-logs admin gate

The logs feed's requireAdmin lives in the local route handler, which the remote
proxy skips when forwarding a request whose nodeId targets a remote node. A hub
user could therefore request /api/logs/global*, /api/logs/global/stream, or
/api/system/log-stream-metrics with x-node-id (or ?nodeId= for the SSE transport)
pointing at a remote node and have it served as the node-proxy admin on the far
side, sidestepping the gate entirely.

Add these paths to HUB_ONLY_PREFIXES so hubOnlyGuard rejects a remote nodeId with
403 before the proxy runs, matching the existing protection on audit-log,
scheduled-tasks, and notification-routes. Add regression tests covering the
collection path, the SSE sub-path (both the x-node-id header and the ?nodeId=
query transport), and the stream-metrics endpoint.

backend/src/__tests__/global-logs-metrics.test.ts

@@ -0,0 +1,71 @@
+import { describe, it, expect, beforeEach } from 'vitest';
+import { GlobalLogsMetrics } from '../services/GlobalLogsMetrics';
+import { mapWithConcurrency } from '../routes/metrics';
+
+describe('GlobalLogsMetrics', () => {
+  beforeEach(() => GlobalLogsMetrics.resetForTests());
+
+  it('starts at all-zero', () => {
+    const s = GlobalLogsMetrics.snapshot();
+    expect(s.active_sse_connections).toBe(0);
+    expect(s.sse_connections_total).toBe(0);
+    expect(s.lines_streamed_total).toBe(0);
+  });
+
+  it('openConnection bumps both the gauge and the total; closeConnection drops only the gauge', () => {
+    GlobalLogsMetrics.openConnection();
+    GlobalLogsMetrics.openConnection();
+    expect(GlobalLogsMetrics.snapshot().active_sse_connections).toBe(2);
+    expect(GlobalLogsMetrics.snapshot().sse_connections_total).toBe(2);
+
+    GlobalLogsMetrics.closeConnection();
+    expect(GlobalLogsMetrics.snapshot().active_sse_connections).toBe(1);
+    // The monotonic total is untouched by a close.
+    expect(GlobalLogsMetrics.snapshot().sse_connections_total).toBe(2);
+  });
+
+  it('clamps the active gauge at zero on an unbalanced close', () => {
+    GlobalLogsMetrics.closeConnection();
+    GlobalLogsMetrics.closeConnection();
+    expect(GlobalLogsMetrics.snapshot().active_sse_connections).toBe(0);
+  });
+
+  it('increments a monotonic counter by an explicit amount', () => {
+    GlobalLogsMetrics.increment('lines_streamed_total', 5);
+    GlobalLogsMetrics.increment('lines_streamed_total');
+    expect(GlobalLogsMetrics.snapshot().lines_streamed_total).toBe(6);
+  });
+
+  it('snapshot returns a copy, not a live reference', () => {
+    const s = GlobalLogsMetrics.snapshot();
+    s.lines_streamed_total = 999;
+    expect(GlobalLogsMetrics.snapshot().lines_streamed_total).toBe(0);
+  });
+});
+
+describe('mapWithConcurrency', () => {
+  it('runs every item exactly once', async () => {
+    const seen: number[] = [];
+    await mapWithConcurrency([1, 2, 3, 4, 5], 2, async (n) => { seen.push(n); });
+    expect(seen.sort((a, b) => a - b)).toEqual([1, 2, 3, 4, 5]);
+  });
+
+  it('never exceeds the concurrency limit in flight', async () => {
+    let inFlight = 0;
+    let peak = 0;
+    await mapWithConcurrency([...Array(20).keys()], 4, async () => {
+      inFlight += 1;
+      peak = Math.max(peak, inFlight);
+      await new Promise(r => setTimeout(r, 2));
+      inFlight -= 1;
+    });
+    expect(peak).toBeLessThanOrEqual(4);
+    expect(peak).toBeGreaterThan(1); // proves it actually parallelizes
+  });
+
+  it('is a no-op on an empty array', async () => {
+    let calls = 0;
+    await mapWithConcurrency([], 4, async () => { calls += 1; });
+    expect(calls).toBe(0);
+  });
+});

backend/src/__tests__/hub-only-guard.test.ts

@@ -120,6 +120,51 @@ describe('hubOnlyGuard', () => {
     expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT');
   });
 
+  // Regression for the Global Observability admin gate: the logs feed's
+  // `requireAdmin` lives in the local route handler, which the proxy skips when
+  // forwarding a remote nodeId. Without these prefixes the guard would let the
+  // request through to the proxy and a hub user could read a remote node's logs
+  // as the node-proxy admin. Cover the collection, the SSE sub-path, and the
+  // stream-metrics endpoint, with and without a trailing slash.
+  it('rejects /api/logs/global with 403 when nodeId targets a remote node', async () => {
+    const res = await request(app)
+      .get('/api/logs/global')
+      .set('Authorization', authHeader)
+      .set('x-node-id', String(remoteNodeId));
+
+    expect(res.status).toBe(403);
+    expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT');
+  });
+
+  it('rejects /api/logs/global/stream with 403 when nodeId targets a remote node', async () => {
+    const res = await request(app)
+      .get('/api/logs/global/stream')
+      .set('Authorization', authHeader)
+      .set('x-node-id', String(remoteNodeId));
+
+    expect(res.status).toBe(403);
+    expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT');
+  });
+
+  it('rejects /api/logs/global/stream via the ?nodeId= query param (SSE transport)', async () => {
+    const res = await request(app)
+      .get(`/api/logs/global/stream?nodeId=${remoteNodeId}`)
+      .set('Authorization', authHeader);
+
+    expect(res.status).toBe(403);
+    expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT');
+  });
+
+  it('rejects /api/system/log-stream-metrics with 403 when nodeId targets a remote node', async () => {
+    const res = await request(app)
+      .get('/api/system/log-stream-metrics')
+      .set('Authorization', authHeader)
+      .set('x-node-id', String(remoteNodeId));
+
+    expect(res.status).toBe(403);
+    expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT');
+  });
+
   it('does not interfere with non-hub paths even when nodeId targets a remote node', async () => {
     // /api/stacks is not hub-only and should be forwarded by the proxy.
     // The exact upstream-error status is not the contract here; what

backend/src/__tests__/log-parsing.test.ts

@@ -5,8 +5,18 @@ import {
   detectLogLevel,
   stripControlChars,
   demuxDockerLog,
+  createFrameDemuxer,
 } from '../utils/log-parsing';
 
+/** Build one Docker multiplexed frame: [streamType, 0,0,0, len(BE)] + payload. */
+function frame(streamType: 1 | 2, payload: string): Buffer {
+  const body = Buffer.from(payload);
+  const header = Buffer.alloc(8);
+  header[0] = streamType;
+  header.writeUInt32BE(body.length, 4);
+  return Buffer.concat([header, body]);
+}
+
 // ── normalizeContainerName ──────────────────────────────────────────────────
 
 describe('normalizeContainerName', () => {
@@ -192,27 +202,18 @@ describe('demuxDockerLog', () => {
     const buf = Buffer.from('line1\nline2\n');
     const lines: Array<{ line: string; source: string }> = [];
     demuxDockerLog(buf, true, (line, source) => lines.push({ line, source }));
+    // A trailing newline does not produce a spurious empty final line.
     expect(lines).toEqual([
       { line: 'line1', source: 'STDOUT' },
       { line: 'line2', source: 'STDOUT' },
-      { line: '', source: 'STDOUT' },
     ]);
   });
 
   it('parses multiplexed STDOUT frame', () => {
-    // Build a frame: [1, 0, 0, 0, <length BE>, ...payload]
-    const payload = Buffer.from('hello stdout\n');
-    const header = Buffer.alloc(8);
-    header[0] = 1; // STDOUT
-    header.writeUInt32BE(payload.length, 4);
-    const buf = Buffer.concat([header, payload]);
-
+    const buf = frame(1, 'hello stdout\n');
     const lines: Array<{ line: string; source: string }> = [];
     demuxDockerLog(buf, false, (line, source) => lines.push({ line, source }));
-    expect(lines).toEqual([
-      { line: 'hello stdout', source: 'STDOUT' },
-      { line: '', source: 'STDOUT' },
-    ]);
+    expect(lines).toEqual([{ line: 'hello stdout', source: 'STDOUT' }]);
   });
 
   it('parses multiplexed STDERR frame', () => {
@@ -254,3 +255,70 @@ describe('demuxDockerLog', () => {
     expect(lines[0]).toEqual({ line: 'cleantext', source: 'STDOUT' });
   });
 });
+
+// ── createFrameDemuxer (stateful, survives chunk boundaries) ─────────────────
+
+describe('createFrameDemuxer', () => {
+  function collect() {
+    const lines: Array<{ line: string; source: string }> = [];
+    let errors = 0;
+    const d = createFrameDemuxer(false, (line, source) => lines.push({ line, source }), () => { errors += 1; });
+    return { lines, push: d.push, flush: d.flush, errors: () => errors };
+  }
+
+  it('reassembles a frame whose 8-byte header is split across two chunks', () => {
+    const full = frame(1, 'split header\n');
+    const c = collect();
+    c.push(full.subarray(0, 3)); // first 3 bytes of the header
+    c.push(full.subarray(3));    // remainder
+    c.flush();
+    expect(c.lines).toEqual([{ line: 'split header', source: 'STDOUT' }]);
+  });
+
+  it('reassembles a frame whose payload is split across two chunks', () => {
+    const full = frame(2, 'partial payload\n');
+    const c = collect();
+    c.push(full.subarray(0, 12)); // header + start of payload
+    c.push(full.subarray(12));
+    c.flush();
+    expect(c.lines).toEqual([{ line: 'partial payload', source: 'STDERR' }]);
+  });
+
+  it('joins a single log line split across two frames', () => {
+    const c = collect();
+    c.push(frame(1, 'hello ')); // no newline yet
+    c.push(frame(1, 'world\n'));
+    c.flush();
+    expect(c.lines).toEqual([{ line: 'hello world', source: 'STDOUT' }]);
+  });
+
+  it('keeps interleaved STDOUT/STDERR partial lines separate', () => {
+    const c = collect();
+    c.push(frame(1, 'out-part '));
+    c.push(frame(2, 'err-part '));
+    c.push(frame(1, 'out-end\n'));
+    c.push(frame(2, 'err-end\n'));
+    c.flush();
+    expect(c.lines).toEqual([
+      { line: 'out-part out-end', source: 'STDOUT' },
+      { line: 'err-part err-end', source: 'STDERR' },
+    ]);
+  });
+
+  it('flushes a buffered partial line with no trailing newline on stream end', () => {
+    const c = collect();
+    c.push(frame(1, 'no newline'));
+    expect(c.lines).toEqual([]); // held until flush
+    c.flush();
+    expect(c.lines).toEqual([{ line: 'no newline', source: 'STDOUT' }]);
+  });
+
+  it('counts a malformed frame header and resyncs instead of stalling', () => {
+    const c = collect();
+    // A stray byte > 2 where a stream type is expected, then a valid frame.
+    c.push(Buffer.concat([Buffer.from([0x07]), frame(1, 'recovered\n')]));
+    c.flush();
+    expect(c.errors()).toBeGreaterThanOrEqual(1);
+    expect(c.lines).toContainEqual({ line: 'recovered', source: 'STDOUT' });
+  });
+});

backend/src/__tests__/managed-containers.test.ts

@@ -0,0 +1,35 @@
+import { describe, it, expect } from 'vitest';
+import path from 'path';
+import { isManagedByComposeDir } from '../utils/managed-containers';
+
+const COMPOSE_DIR = path.resolve('/srv/compose');
+
+function withWorkingDir(workingDir?: string): { Labels?: Record<string, string> } {
+  return workingDir === undefined
+    ? {}
+    : { Labels: { 'com.docker.compose.project.working_dir': workingDir } };
+}
+
+describe('isManagedByComposeDir', () => {
+  it('treats a container in a subdirectory of COMPOSE_DIR as managed', () => {
+    expect(isManagedByComposeDir(withWorkingDir(path.join(COMPOSE_DIR, 'web')), COMPOSE_DIR)).toBe(true);
+  });
+
+  it('treats a container launched from the COMPOSE_DIR root as managed', () => {
+    expect(isManagedByComposeDir(withWorkingDir(COMPOSE_DIR), COMPOSE_DIR)).toBe(true);
+  });
+
+  it('treats a container outside COMPOSE_DIR as unmanaged', () => {
+    expect(isManagedByComposeDir(withWorkingDir('/opt/other/stack'), COMPOSE_DIR)).toBe(false);
+  });
+
+  it('does not match a sibling directory that shares the COMPOSE_DIR prefix', () => {
+    // /srv/compose-extra must not be considered inside /srv/compose.
+    expect(isManagedByComposeDir(withWorkingDir(`${COMPOSE_DIR}-extra/web`), COMPOSE_DIR)).toBe(false);
+  });
+
+  it('treats a container with no compose working-dir label as unmanaged', () => {
+    expect(isManagedByComposeDir(withWorkingDir(undefined), COMPOSE_DIR)).toBe(false);
+    expect(isManagedByComposeDir({ Labels: {} }, COMPOSE_DIR)).toBe(false);
+  });
+});

backend/src/__tests__/metrics-routes.test.ts

@@ -109,12 +109,59 @@ describe('GET /api/system/cache-stats', () => {
   });
 });
 
+describe('GET /api/logs/global (poll snapshot)', () => {
+  it('rejects unauthenticated requests with 401', async () => {
+    const res = await request(app).get('/api/logs/global');
+    expect(res.status).toBe(401);
+  });
+
+  it('rejects non-admin users with 403', async () => {
+    const res = await request(app).get('/api/logs/global').set('Cookie', viewerCookie);
+    expect(res.status).toBe(403);
+  });
+
+  it('admin gets a non-4xx response', async () => {
+    // Reaches the Docker daemon; 500 is acceptable in CI without Docker. We
+    // only prove the admin gate + routing, not the daemon read.
+    const res = await request(app).get('/api/logs/global').set('Cookie', adminCookie);
+    expect([200, 500]).toContain(res.status);
+    if (res.status === 200) expect(Array.isArray(res.body)).toBe(true);
+  });
+});
+
+describe('GET /api/system/log-stream-metrics', () => {
+  it('rejects unauthenticated requests with 401', async () => {
+    const res = await request(app).get('/api/system/log-stream-metrics');
+    expect(res.status).toBe(401);
+  });
+
+  it('rejects non-admin users with 403', async () => {
+    const res = await request(app).get('/api/system/log-stream-metrics').set('Cookie', viewerCookie);
+    expect(res.status).toBe(403);
+  });
+
+  it('returns the counter snapshot for admin', async () => {
+    const res = await request(app).get('/api/system/log-stream-metrics').set('Cookie', adminCookie);
+    expect(res.status).toBe(200);
+    expect(res.body).toHaveProperty('active_sse_connections');
+    expect(res.body).toHaveProperty('lines_streamed_total');
+    expect(res.body).toHaveProperty('stream_attach_errors_total');
+  });
+});
+
 describe('GET /api/logs/global/stream', () => {
   it('rejects unauthenticated requests with 401', async () => {
     const res = await request(app).get('/api/logs/global/stream');
     expect(res.status).toBe(401);
   });
 
+  it('rejects non-admin users with 403 before opening the stream', async () => {
+    // requireAdmin runs before flushHeaders, so a viewer gets a clean JSON 403
+    // rather than a half-open event-stream.
+    const res = await request(app).get('/api/logs/global/stream').set('Cookie', viewerCookie);
+    expect(res.status).toBe(403);
+  });
+
   it('sets SSE response headers for authenticated users', async () => {
     // The SSE handler writes headers immediately then keeps the connection
     // open. supertest .end() after we see the headers lets Express flush