From 18762fa74b7ce2d1516ad87c1565cbc9f177328f Mon Sep 17 00:00:00 2001 From: Anso Date: Mon, 1 Jun 2026 13:16:56 -0400 Subject: [PATCH] fix(registries): keep registry endpoints local to each instance (#1267) Registry credentials are stored and resolved per instance, and the UI only ever calls registry endpoints with localOnly. The endpoints were not on the hub-only proxy list, so a scripted request carrying an x-node-id for a remote node would be forwarded by the remote proxy, carrying a plaintext registry secret to (or reading stored credentials from) the remote instance. Add /api/registries/ to HUB_ONLY_PREFIXES so a remote-targeted registry request is rejected with 403 before the proxy can forward it, matching the existing defense-in-depth for the audit log, schedules, and the global logs feed. Add regression tests for the collection, a sub-path, and the local pass-through. Correct a stale 409 reference in the guard comments and test header to the 403 the guard actually returns. --- backend/src/__tests__/hub-only-guard.test.ts | 38 +++++++++++++++++++- backend/src/helpers/proxyExemptPaths.ts | 23 +++++++----- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/backend/src/__tests__/hub-only-guard.test.ts b/backend/src/__tests__/hub-only-guard.test.ts index 7303f973..e30ab9e2 100644 --- a/backend/src/__tests__/hub-only-guard.test.ts +++ b/backend/src/__tests__/hub-only-guard.test.ts @@ -4,7 +4,7 @@ * Hub-only paths (e.g. /api/scheduled-tasks, /api/audit-log, * /api/notification-routes) manage state owned by the local hub. When a * request carries `x-node-id` for a remote node, the guard must reject - * with 409 before the remote proxy forwards it. Without this guard, a + * with 403 before the remote proxy forwards it. Without this guard, a * scripted client could trick the proxy into running hub-level operations * on a remote instance, crossing a node-authority boundary that the UI * promises will not happen. @@ -165,6 +165,42 @@ describe('hubOnlyGuard', () => { expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT'); }); + // Regression: registry credentials are stored and managed per instance. + // Without the guard, a scripted `x-node-id: ` request would be + // forwarded by the proxy and carry a plaintext secret to the remote. Cover + // the collection (no trailing slash, the form a bare startsWith would leak) + // and a sub-path, plus the local pass-through so a future over-broadening of + // the prefix that 403s legitimate local registry management is caught. + it('rejects /api/registries with 403 when nodeId targets a remote node', async () => { + const res = await request(app) + .get('/api/registries') + .set('Authorization', authHeader) + .set('x-node-id', String(remoteNodeId)); + + expect(res.status).toBe(403); + expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT'); + }); + + it('rejects a registry sub-path (/api/registries/1/test) with 403 when nodeId targets a remote node', async () => { + const res = await request(app) + .post('/api/registries/1/test') + .set('Authorization', authHeader) + .set('x-node-id', String(remoteNodeId)); + + expect(res.status).toBe(403); + expect(res.body?.code).toBe('HUB_ONLY_ENDPOINT'); + }); + + it('lets /api/registries through to the local handler when no nodeId is set', async () => { + const res = await request(app) + .get('/api/registries') + .set('Authorization', authHeader); + + // The local handler may return 200 or a tier/role gate 403; what matters is + // the guard did not reject with HUB_ONLY_ENDPOINT. + expect(res.body?.code).not.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 diff --git a/backend/src/helpers/proxyExemptPaths.ts b/backend/src/helpers/proxyExemptPaths.ts index 32b5a9a7..5eae1c57 100644 --- a/backend/src/helpers/proxyExemptPaths.ts +++ b/backend/src/helpers/proxyExemptPaths.ts @@ -23,14 +23,18 @@ export function isProxyExemptPath(path: string): boolean { return false; } -// Path prefixes that are hub-only: they manage or expose state owned by the -// local hub (centralized audit, fleet schedules, notification routing rules, -// the admin-only aggregated logs feed and its stream counters). Routed to the -// local hub when nodeId resolves to local, but rejected when nodeId resolves -// to a remote node so a script/curl call cannot trick the proxy into -// forwarding hub-only authority across a node boundary. This matters for the -// logs feed in particular: its admin gate lives in the local route handler, -// which the proxy would skip entirely when forwarding a remote nodeId. +// Path prefixes that are hub-only: they must be served on the instance you are +// signed into and never proxied to a remote node. This covers state owned by +// the local hub (centralized audit, fleet schedules, notification routing +// rules, the admin-only aggregated logs feed and its stream counters) and +// private registry credentials, which are stored and managed per instance. +// Routed to the local hub when nodeId resolves to local, but rejected when +// nodeId resolves to a remote node so a script/curl call cannot trick the proxy +// into forwarding the request across a node boundary. This matters for the logs +// feed (its admin gate lives in the local route handler, which the proxy would +// skip when forwarding a remote nodeId) and for registries (a proxied registry +// request would otherwise carry a plaintext secret to, or read stored +// credentials from, the remote). // // Entries are stored with a trailing slash; the matcher accepts the exact // collection path (without the trailing slash) AND any sub-path under it, @@ -42,13 +46,14 @@ export function isProxyExemptPath(path: string): boolean { // useViewNavigationState.ts; this list is the backend defense-in-depth. // // Consumed by: -// - middleware/hubOnlyGuard.ts → 409 when nodeId is remote +// - middleware/hubOnlyGuard.ts → 403 when nodeId is remote export const HUB_ONLY_PREFIXES: readonly string[] = [ '/api/scheduled-tasks/', '/api/audit-log/', '/api/notification-routes/', '/api/logs/global/', '/api/system/log-stream-metrics/', + '/api/registries/', ]; /** Returns true when the path is hub-only and must not be proxied to a remote node. */