diff --git a/.changeset/fn-7647-auth-storage-routes-coordination.md b/.changeset/fn-7647-auth-storage-routes-coordination.md new file mode 100644 index 0000000000..17b22a038f --- /dev/null +++ b/.changeset/fn-7647-auth-storage-routes-coordination.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Route node settings-sync and mesh credential writes through the coordinated auth store to prevent concurrent clobbers. +category: fix +dev: register-settings-sync-routes.ts, register-settings-sync-inbound-routes.ts, and register-mesh-routes.ts now persist received credentials via @fusion/engine createFusionAuthStorage() instead of raw AuthStorage.create(getFusionAuthPath()), sharing FN-7646's reload-before-persist + per-provider locked-merge path over ~/.fusion/agent/auth.json. Adds route-level regression coverage. diff --git a/packages/dashboard/src/__tests__/mesh-routes.test.ts b/packages/dashboard/src/__tests__/mesh-routes.test.ts index 8a2b69988f..6578d93b8b 100644 --- a/packages/dashboard/src/__tests__/mesh-routes.test.ts +++ b/packages/dashboard/src/__tests__/mesh-routes.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { EventEmitter } from "node:events"; import type { Task, TaskStore } from "@fusion/core"; +import { createAuthMaterialSnapshot } from "@fusion/core"; import { request } from "../test-request.js"; import { createServer } from "../server.js"; import type { RuntimeLogger } from "../runtime-logger.js"; @@ -31,6 +32,7 @@ const mockCommitDistributedTaskIdReservation = vi.fn(); const mockAbortDistributedTaskIdReservation = vi.fn(); const mockGetDistributedTaskIdState = vi.fn(); const mockApplyReplicatedTaskCreate = vi.fn(); +const mockApplyAuthMaterialSnapshot = vi.fn(); // Mock GlobalSettingsStore const mockGetSettings = vi.fn().mockResolvedValue({}); @@ -55,10 +57,38 @@ vi.mock("@fusion/core", async () => { getLocalMeshSnapshot: mockGetLocalMeshSnapshot, getSettingsForSync: mockGetSettingsForSync, applyRemoteSettings: mockApplyRemoteSettings, + applyAuthMaterialSnapshot: mockApplyAuthMaterialSnapshot, }; }), }; }); +// FNXC:ProviderAuth 2026-07-07-00:00: FN-7647 routed register-mesh-routes.ts's inline +// auth-material shared-state domain through @fusion/engine's createFusionAuthStorage() instead of +// a raw AuthStorage.create(getFusionAuthPath()). Mock the factory so mesh sync auth-material writes +// are observable and never touch the real ~/.fusion/agent/auth.json during tests; preserve every +// other real @fusion/engine export other routers rely on at module load time. vi.mock factories are +// hoisted above top-level const declarations, so the referenced mocks must be created via +// vi.hoisted to avoid a temporal-dead-zone ReferenceError. +const { mockMeshAuthStorageSet, mockCreateFusionAuthStorage } = vi.hoisted(() => { + const mockMeshAuthStorageSet = vi.fn().mockResolvedValue(undefined); + const mockCreateFusionAuthStorage = vi.fn(() => ({ + set: mockMeshAuthStorageSet, + get: vi.fn(), + getApiKey: vi.fn(), + getOAuthProviders: vi.fn().mockReturnValue([]), + reload: vi.fn(), + })); + return { mockMeshAuthStorageSet, mockCreateFusionAuthStorage }; +}); + +vi.mock("@fusion/engine", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createFusionAuthStorage: mockCreateFusionAuthStorage, + }; +}); + class MockStore extends EventEmitter { getRootDir(): string { return "/tmp/fn-1224"; @@ -703,6 +733,94 @@ describe("POST /api/mesh/sync", () => { ); }); }); + + // ── FN-7647 Symptom Verification: auth-material shared-state sync ───────────── + // Original symptom (FN-7646 class): a raw independent AuthStorage instance can persist a stale + // in-memory snapshot over ~/.fusion/agent/auth.json, wiping a key another process just saved. + // Asserts the mesh sync auth-material domain writes via the coordinated createFusionAuthStorage() + // proxy (not a raw instance) and that an unrelated provider's saved credential survives the write. + describe("auth-material shared-state sync", () => { + beforeEach(() => { + mockApplyAuthMaterialSnapshot.mockReset(); + mockCreateFusionAuthStorage.mockClear(); + mockMeshAuthStorageSet.mockReset().mockResolvedValue(undefined); + }); + + it("writes received auth-material credentials via the coordinated createFusionAuthStorage() proxy", async () => { + mockApplyAuthMaterialSnapshot.mockReturnValue({ + success: true, + authCount: 1, + providerAuth: { anthropic: { type: "api_key", key: "sk-ant-mesh-received" } }, + }); + + const authMaterial = createAuthMaterialSnapshot({ + anthropic: { type: "api_key", key: "sk-ant-mesh-received" }, + }); + + const response = await request( + app, + "POST", + "/api/mesh/sync", + JSON.stringify({ + senderNodeId: "node_remote", + senderNodeUrl: "https://remote.example.com", + knownPeers: [], + timestamp: "2026-04-01T12:00:00.000Z", + sharedState: { authMaterial }, + }), + { "Content-Type": "application/json" } + ); + + expect(response.status).toBe(200); + expect(mockApplyAuthMaterialSnapshot).toHaveBeenCalledWith(authMaterial); + // Proves the write path is the coordinated proxy, not a raw independent AuthStorage instance. + expect(mockCreateFusionAuthStorage).toHaveBeenCalled(); + expect(mockMeshAuthStorageSet).toHaveBeenCalledWith("anthropic", { type: "api_key", key: "sk-ant-mesh-received" }); + }); + + it("concurrent-writer survival: an unrelated provider's saved key is never clobbered by the mesh sync write", async () => { + mockApplyAuthMaterialSnapshot.mockReturnValue({ + success: true, + authCount: 1, + providerAuth: { anthropic: { type: "api_key", key: "sk-ant-mesh-received" } }, + }); + + // Simulate another Fusion instance's provider credential already persisted on disk by having + // the mocked coordinated proxy merge per-provider (as the real reload-before-persist proxy + // does) rather than overwrite the whole file. + const diskState: Record = { + openai: { type: "api_key", key: "sk-openai-from-other-instance" }, + }; + mockMeshAuthStorageSet.mockImplementation(async (providerId: string, credential: unknown) => { + diskState[providerId] = credential; + }); + + const authMaterial = createAuthMaterialSnapshot({ + anthropic: { type: "api_key", key: "sk-ant-mesh-received" }, + }); + + const response = await request( + app, + "POST", + "/api/mesh/sync", + JSON.stringify({ + senderNodeId: "node_remote", + senderNodeUrl: "https://remote.example.com", + knownPeers: [], + timestamp: "2026-04-01T12:00:00.000Z", + sharedState: { authMaterial }, + }), + { "Content-Type": "application/json" } + ); + + expect(response.status).toBe(200); + // The unrelated provider saved by another instance before this handler ran must still be present. + expect(diskState.openai).toEqual({ type: "api_key", key: "sk-openai-from-other-instance" }); + // And the received provider was merged in alongside it — no full-snapshot clobber. + expect(diskState.anthropic).toEqual({ type: "api_key", key: "sk-ant-mesh-received" }); + expect(mockMeshAuthStorageSet).toHaveBeenCalledTimes(1); + }); + }); }); describe("/api/mesh/task-ids routes", () => { diff --git a/packages/dashboard/src/__tests__/routes-nodes-sync-contract.test.ts b/packages/dashboard/src/__tests__/routes-nodes-sync-contract.test.ts index dd2236f683..eea7ad987c 100644 --- a/packages/dashboard/src/__tests__/routes-nodes-sync-contract.test.ts +++ b/packages/dashboard/src/__tests__/routes-nodes-sync-contract.test.ts @@ -62,8 +62,23 @@ vi.mock("@fusion/core", async (importOriginal) => { }; }); -const mockAuthStorageSet = vi.fn(); -const mockAuthStorageGetOAuthProviders = vi.fn().mockReturnValue([]); +// FNXC:ProviderAuth 2026-07-07-00:00: FN-7647 routed register-settings-sync-routes.ts and +// register-settings-sync-inbound-routes.ts through @fusion/engine's createFusionAuthStorage() +// instead of a raw AuthStorage.create(getFusionAuthPath()). Mock the factory (not just the raw +// pi-coding-agent AuthStorage) so this contract matrix never falls through to the real proxy and +// touches the developer's actual ~/.fusion/agent/auth.json during the auth-sync/auth-receive cases. +const { mockAuthStorageSet, mockAuthStorageGetOAuthProviders, mockCreateFusionAuthStorage } = vi.hoisted(() => { + const mockAuthStorageSet = vi.fn(); + const mockAuthStorageGetOAuthProviders = vi.fn().mockReturnValue([]); + const mockCreateFusionAuthStorage = vi.fn(() => ({ + set: mockAuthStorageSet, + get: vi.fn(), + getApiKey: vi.fn(), + getOAuthProviders: mockAuthStorageGetOAuthProviders, + reload: vi.fn(), + })); + return { mockAuthStorageSet, mockAuthStorageGetOAuthProviders, mockCreateFusionAuthStorage }; +}); vi.mock("@earendil-works/pi-coding-agent", () => { return { @@ -79,6 +94,14 @@ vi.mock("@earendil-works/pi-coding-agent", () => { }; }); +vi.mock("@fusion/engine", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createFusionAuthStorage: mockCreateFusionAuthStorage, + }; +}); + class MockStore extends EventEmitter { getRootDir(): string { return "/tmp/fn-4755-test"; diff --git a/packages/dashboard/src/__tests__/routes-nodes-sync.test.ts b/packages/dashboard/src/__tests__/routes-nodes-sync.test.ts index 46e11323f1..306f26cb25 100644 --- a/packages/dashboard/src/__tests__/routes-nodes-sync.test.ts +++ b/packages/dashboard/src/__tests__/routes-nodes-sync.test.ts @@ -71,9 +71,24 @@ vi.mock("@fusion/core", async (importOriginal) => { }); // ── Mock AuthStorage ─────────────────────────────────────────────────── +// FNXC:ProviderAuth 2026-07-07-00:00: FN-7647 routed register-settings-sync-routes.ts and +// register-settings-sync-inbound-routes.ts through @fusion/engine's createFusionAuthStorage() +// instead of a raw AuthStorage.create(getFusionAuthPath()). Mock the factory (not the raw +// pi-coding-agent AuthStorage) so mockAuthStorageSet still observes the coordinated write path, +// and preserve every other real @fusion/engine export other routers rely on at module load time. -const mockAuthStorageSet = vi.fn(); -const mockAuthStorageGetOAuthProviders = vi.fn().mockReturnValue([]); +const { mockAuthStorageSet, mockAuthStorageGetOAuthProviders, mockCreateFusionAuthStorage } = vi.hoisted(() => { + const mockAuthStorageSet = vi.fn(); + const mockAuthStorageGetOAuthProviders = vi.fn().mockReturnValue([]); + const mockCreateFusionAuthStorage = vi.fn(() => ({ + set: mockAuthStorageSet, + get: vi.fn(), + getApiKey: vi.fn(), + getOAuthProviders: mockAuthStorageGetOAuthProviders, + reload: vi.fn(), + })); + return { mockAuthStorageSet, mockAuthStorageGetOAuthProviders, mockCreateFusionAuthStorage }; +}); vi.mock("@earendil-works/pi-coding-agent", () => { return { @@ -89,6 +104,14 @@ vi.mock("@earendil-works/pi-coding-agent", () => { }; }); +vi.mock("@fusion/engine", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createFusionAuthStorage: mockCreateFusionAuthStorage, + }; +}); + // ── Mock Store ──────────────────────────────────────────────────────── class MockStore extends EventEmitter { @@ -1072,6 +1095,11 @@ describe("Node settings sync routes", () => { lastSyncedAt: expect.any(String), }), ); + // FN-7647: pull-mode credential writes go through the coordinated createFusionAuthStorage() + // proxy, not a raw independent AuthStorage instance. (applyAuthMaterialSnapshot resolves the + // default beforeEach mock here — the anthropic credential — since this test doesn't override it.) + expect(mockCreateFusionAuthStorage).toHaveBeenCalled(); + expect(mockAuthStorageSet).toHaveBeenCalledWith("anthropic", { type: "api_key", key: "sk-ant-received" }); }); it("emits structured redacted diagnostics for pull-mode auth sync", async () => { @@ -1420,6 +1448,82 @@ describe("Node settings sync routes", () => { expect(res.body.receivedProviders).toContain("anthropic"); }); + // ── FN-7647 Symptom Verification ───────────────────────────────── + // Original symptom (FN-7646 class): a raw independent AuthStorage instance can persist a + // stale in-memory snapshot over ~/.fusion/agent/auth.json, wiping a key another process just + // saved. This asserts the inbound handler writes through the coordinated + // createFusionAuthStorage() proxy (not a raw instance) and that per-provider writes never + // clobber an unrelated provider's credential already present on disk. + it("FN-7647: persists received credentials via the coordinated createFusionAuthStorage() proxy", async () => { + const localNode = createMockLocalNode(); + mockListNodes.mockResolvedValue([localNode]); + + const res = await request( + app, + "POST", + "/api/settings/auth-receive", + JSON.stringify({ + authMaterial: { + version: 1, + exportedAt: "2026-04-14T10:00:00.000Z", + checksum: "auth-checksum", + payload: { providerAuth: { anthropic: { type: "api_key", key: "sk-ant-received" } } }, + }, + sourceNodeId: "node-remote-001", + timestamp: "2026-04-14T10:00:00.000Z", + }), + { "content-type": "application/json", "Authorization": `Bearer ${localNode.apiKey}` }, + ); + + expect(res.status).toBe(200); + // Proves the write path is the coordinated proxy, not a raw independent AuthStorage instance. + expect(mockCreateFusionAuthStorage).toHaveBeenCalled(); + expect(mockAuthStorageSet).toHaveBeenCalledWith("anthropic", { type: "api_key", key: "sk-ant-received" }); + }); + + it("FN-7647: concurrent-writer survival — an unrelated provider's saved key is never clobbered by this handler's write", async () => { + const localNode = createMockLocalNode(); + mockListNodes.mockResolvedValue([localNode]); + + // Simulate another Fusion instance's provider credential already persisted on disk by having + // the mocked coordinated proxy merge per-provider (as the real reload-before-persist proxy does) + // rather than overwrite the whole file. `openai` was saved by a concurrent instance and must + // still be observable after this handler's write — this handler only ever calls .set() for the + // providers it received (anthropic), never a whole-snapshot overwrite that would drop `openai`. + const diskState: Record = { + openai: { type: "api_key", key: "sk-openai-from-other-instance" }, + }; + mockAuthStorageSet.mockImplementation(async (providerId: string, credential: unknown) => { + diskState[providerId] = credential; + }); + + const res = await request( + app, + "POST", + "/api/settings/auth-receive", + JSON.stringify({ + authMaterial: { + version: 1, + exportedAt: "2026-04-14T10:00:00.000Z", + checksum: "auth-checksum", + payload: { providerAuth: { anthropic: { type: "api_key", key: "sk-ant-received" } } }, + }, + sourceNodeId: "node-remote-001", + timestamp: "2026-04-14T10:00:00.000Z", + }), + { "content-type": "application/json", "Authorization": `Bearer ${localNode.apiKey}` }, + ); + + expect(res.status).toBe(200); + // The unrelated provider saved by another instance before this handler ran must still be present. + expect(diskState.openai).toEqual({ type: "api_key", key: "sk-openai-from-other-instance" }); + // And the received provider was merged in alongside it — no full-snapshot clobber. + expect(diskState.anthropic).toEqual({ type: "api_key", key: "sk-ant-received" }); + // .set() was only invoked for the received provider — never a whole-file overwrite call. + expect(mockAuthStorageSet).toHaveBeenCalledTimes(1); + expect(mockAuthStorageSet).toHaveBeenCalledWith("anthropic", { type: "api_key", key: "sk-ant-received" }); + }); + it("returns 401 when auth header is missing", async () => { const res = await request( app, diff --git a/packages/dashboard/src/routes/register-mesh-routes.ts b/packages/dashboard/src/routes/register-mesh-routes.ts index 6713378abe..cc77a6d6ae 100644 --- a/packages/dashboard/src/routes/register-mesh-routes.ts +++ b/packages/dashboard/src/routes/register-mesh-routes.ts @@ -1,3 +1,4 @@ +import { createFusionAuthStorage } from "@fusion/engine"; import { ApiError, badRequest } from "../api-error.js"; import type { ApiRouteRegistrar } from "./types.js"; import { fetchFromRemoteNode } from "./register-settings-sync-helpers.js"; @@ -531,9 +532,16 @@ export const registerMeshRoutes: ApiRouteRegistrar = (ctx) => { if (!sharedState.authMaterial) return; validateSnapshotEnvelope(sharedState.authMaterial); const applied = central.applyAuthMaterialSnapshot(sharedState.authMaterial as Parameters[0]); - const { AuthStorage } = await import("@earendil-works/pi-coding-agent"); - const { getFusionAuthPath } = await import("../auth-paths.js"); - const authStorage = AuthStorage.create(getFusionAuthPath()); + /* + * FNXC:ProviderAuth 2026-07-07-00:00: + * Dashboard sync/mesh credential writes must go through the coordinated createFusionAuthStorage() + * proxy (reload-before-persist, supplemental-credential sync, logout suppression, Anthropic aliasing) + * instead of a raw AuthStorage.create(getFusionAuthPath()) instance, so concurrent Fusion processes + * sharing ~/.fusion/agent/auth.json do not clobber each other's saved provider keys. FN-7647, + * follow-up to FN-7646's engine-side hardening. Uses a static top-level import (not dynamic + * import) per FN-3049's bundler-safety rule against dynamic engine imports (`await` + `import(...)` of the engine package). + */ + const authStorage = createFusionAuthStorage(); for (const [providerId, credential] of Object.entries(applied.providerAuth)) { if (credential.type === "api_key" && credential.key) { authStorage.set(providerId, { type: "api_key", key: credential.key }); diff --git a/packages/dashboard/src/routes/register-settings-sync-inbound-routes.ts b/packages/dashboard/src/routes/register-settings-sync-inbound-routes.ts index 15a021ed4e..beec7354d3 100644 --- a/packages/dashboard/src/routes/register-settings-sync-inbound-routes.ts +++ b/packages/dashboard/src/routes/register-settings-sync-inbound-routes.ts @@ -1,7 +1,7 @@ import { isMovedSettingsKey } from "@fusion/core"; +import { createFusionAuthStorage } from "@fusion/engine"; import { ApiError, badRequest } from "../api-error.js"; import { invalidateAllGlobalSettingsCaches } from "../project-store-resolver.js"; -import { getFusionAuthPath } from "../auth-paths.js"; import { readStoredAuthProvidersFromDisk, toProviderAuthEntries } from "./register-settings-sync-helpers.js"; import type { ApiRouteRegistrar } from "./types.js"; @@ -228,9 +228,16 @@ export const registerSettingsSyncInboundRoutes: ApiRouteRegistrar = (ctx) => { throw badRequest("Missing required field: timestamp"); } - // Import AuthStorage and write credentials - const { AuthStorage } = await import("@earendil-works/pi-coding-agent"); - const authStorage = AuthStorage.create(getFusionAuthPath()); + /* + * FNXC:ProviderAuth 2026-07-07-00:00: + * Dashboard sync/mesh credential writes must go through the coordinated createFusionAuthStorage() + * proxy (reload-before-persist, supplemental-credential sync, logout suppression, Anthropic aliasing) + * instead of a raw AuthStorage.create(getFusionAuthPath()) instance, so concurrent Fusion processes + * sharing ~/.fusion/agent/auth.json do not clobber each other's saved provider keys. FN-7647, + * follow-up to FN-7646's engine-side hardening. Uses a static top-level import (not dynamic + * import) per FN-3049's bundler-safety rule against dynamic engine imports (`await` + `import(...)` of the engine package). + */ + const authStorage = createFusionAuthStorage(); const applyResult = central.applyAuthMaterialSnapshot(authMaterial); const receivedProviders: string[] = []; diff --git a/packages/dashboard/src/routes/register-settings-sync-routes.ts b/packages/dashboard/src/routes/register-settings-sync-routes.ts index 1c71b5493f..3d2ceba7cb 100644 --- a/packages/dashboard/src/routes/register-settings-sync-routes.ts +++ b/packages/dashboard/src/routes/register-settings-sync-routes.ts @@ -1,8 +1,8 @@ import type { ProjectSettings } from "@fusion/core"; import { isMovedSettingsKey } from "@fusion/core"; +import { createFusionAuthStorage } from "@fusion/engine"; import { basename } from "node:path"; import { ApiError, badRequest, notFound } from "../api-error.js"; -import { getFusionAuthPath } from "../auth-paths.js"; import { invalidateAllGlobalSettingsCaches } from "../project-store-resolver.js"; import { classifySyncStatusDenialReason, @@ -501,9 +501,16 @@ export const registerSettingsSyncRoutes: ApiRouteRegistrar = (ctx) => { const localPeerInfo = await central.getLocalPeerInfo(); const timestamp = new Date().toISOString(); - // Import AuthStorage - const { AuthStorage } = await import("@earendil-works/pi-coding-agent"); - const authStorage = AuthStorage.create(getFusionAuthPath()); + /* + * FNXC:ProviderAuth 2026-07-07-00:00: + * Dashboard sync/mesh credential writes must go through the coordinated createFusionAuthStorage() + * proxy (reload-before-persist, supplemental-credential sync, logout suppression, Anthropic aliasing) + * instead of a raw AuthStorage.create(getFusionAuthPath()) instance, so concurrent Fusion processes + * sharing ~/.fusion/agent/auth.json do not clobber each other's saved provider keys. FN-7647, + * follow-up to FN-7646's engine-side hardening. Uses a static top-level import (not dynamic + * import) per FN-3049's bundler-safety rule against dynamic engine imports (`await` + `import(...)` of the engine package). + */ + const authStorage = createFusionAuthStorage(); if (direction === "push") { const allProviders = await readStoredAuthProvidersFromDisk();