diff --git a/src/dao/BearerKeyDao.ts b/src/dao/BearerKeyDao.ts index 56738e2..812883b 100644 --- a/src/dao/BearerKeyDao.ts +++ b/src/dao/BearerKeyDao.ts @@ -24,7 +24,10 @@ export class BearerKeyDaoImpl extends JsonFileBaseDao implements BearerKeyDao { private async loadKeysWithMigration(): Promise { const settings = await this.loadSettings(); - if (Array.isArray(settings.bearerKeys) && settings.bearerKeys.length > 0) { + // Treat an existing array (including an empty array) as already migrated. + // Otherwise, when there are no configured keys, we'd rewrite mcp_settings.json + // on every request, which also clears the global settings cache. + if (Array.isArray(settings.bearerKeys)) { return settings.bearerKeys; } diff --git a/tests/dao/bearerKeyDao.test.ts b/tests/dao/bearerKeyDao.test.ts new file mode 100644 index 0000000..be23b6b --- /dev/null +++ b/tests/dao/bearerKeyDao.test.ts @@ -0,0 +1,97 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +import { BearerKeyDaoImpl } from '../../src/dao/BearerKeyDao.js'; + +const writeSettings = (settingsPath: string, settings: unknown): void => { + fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2), 'utf8'); +}; + +describe('BearerKeyDaoImpl migration + settings caching behavior', () => { + let tmpDir: string; + let settingsPath: string; + let originalSettingsEnv: string | undefined; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcphub-bearer-keys-')); + settingsPath = path.join(tmpDir, 'mcp_settings.json'); + + originalSettingsEnv = process.env.MCPHUB_SETTING_PATH; + process.env.MCPHUB_SETTING_PATH = settingsPath; + }); + + afterEach(() => { + if (originalSettingsEnv === undefined) { + delete process.env.MCPHUB_SETTING_PATH; + } else { + process.env.MCPHUB_SETTING_PATH = originalSettingsEnv; + } + + try { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } catch { + // ignore cleanup errors + } + }); + + it('does not rewrite settings when bearerKeys exists as an empty array', async () => { + writeSettings(settingsPath, { + mcpServers: {}, + users: [], + systemConfig: { + routing: { + enableBearerAuth: false, + bearerAuthKey: '', + }, + }, + bearerKeys: [], + }); + + const writeSpy = jest.spyOn(fs, 'writeFileSync'); + + const dao = new BearerKeyDaoImpl(); + const enabled1 = await dao.findEnabled(); + const enabled2 = await dao.findEnabled(); + + expect(enabled1).toEqual([]); + expect(enabled2).toEqual([]); + + // The DAO should NOT persist anything because bearerKeys already exists. + expect(writeSpy).not.toHaveBeenCalled(); + + writeSpy.mockRestore(); + }); + + it('migrates legacy bearerAuthKey only once', async () => { + writeSettings(settingsPath, { + mcpServers: {}, + users: [], + systemConfig: { + routing: { + enableBearerAuth: true, + bearerAuthKey: 'legacy-token', + }, + }, + // bearerKeys is intentionally missing to trigger migration + }); + + const writeSpy = jest.spyOn(fs, 'writeFileSync'); + + const dao = new BearerKeyDaoImpl(); + + const enabled1 = await dao.findEnabled(); + expect(enabled1).toHaveLength(1); + expect(enabled1[0].token).toBe('legacy-token'); + expect(enabled1[0].enabled).toBe(true); + + const enabled2 = await dao.findEnabled(); + expect(enabled2).toHaveLength(1); + expect(enabled2[0].token).toBe('legacy-token'); + + // One write for the migration, no further writes on subsequent reads. + expect(writeSpy).toHaveBeenCalledTimes(1); + + writeSpy.mockRestore(); + }); +});