mirror of
https://github.com/hellodigua/ChatLab.git
synced 2026-10-26 18:34:51 +08:00
fix(runtime): harden migration and cleanup safety
This commit is contained in:
1 parent
43c9229935
commit
7bdd1fe751
9 files changed
+227
-30
No files matched your search
@@ -7,7 +7,12 @@ import test from 'node:test'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import type { AuthProfilesData } from './auth-profiles'
|
||||
|
||||
function runAuthProfileWriterProcess(homeDir: string, workerIndex: number, count: number): Promise<void> {
|
||||
function runAuthProfileWriterProcess(
|
||||
homeDir: string,
|
||||
workerIndex: number,
|
||||
count: number,
|
||||
uniqueBaseName?: string
|
||||
): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const child = spawn(
|
||||
process.execPath,
|
||||
@@ -15,13 +20,18 @@ function runAuthProfileWriterProcess(homeDir: string, workerIndex: number, count
|
||||
'--eval',
|
||||
`
|
||||
require('tsx/cjs/api').register()
|
||||
const { writeAuthProfile } = require(process.env.CHATLAB_AUTH_PROFILES_MODULE)
|
||||
const { writeAuthProfile, writeAuthProfileWithUniqueName } = require(process.env.CHATLAB_AUTH_PROFILES_MODULE)
|
||||
for (let index = 0; index < Number(process.env.CHATLAB_PROFILE_COUNT); index++) {
|
||||
writeAuthProfile(\`worker-\${process.env.CHATLAB_WORKER_INDEX}-\${index}\`, {
|
||||
const profile = {
|
||||
type: 'api_key',
|
||||
provider: 'openai',
|
||||
key: \`secret-\${process.env.CHATLAB_WORKER_INDEX}-\${index}\`,
|
||||
})
|
||||
}
|
||||
if (process.env.CHATLAB_UNIQUE_BASE_NAME) {
|
||||
writeAuthProfileWithUniqueName(process.env.CHATLAB_UNIQUE_BASE_NAME, profile)
|
||||
} else {
|
||||
writeAuthProfile(\`worker-\${process.env.CHATLAB_WORKER_INDEX}-\${index}\`, profile)
|
||||
}
|
||||
}
|
||||
`,
|
||||
],
|
||||
@@ -31,6 +41,7 @@ function runAuthProfileWriterProcess(homeDir: string, workerIndex: number, count
|
||||
CHATLAB_AUTH_PROFILES_MODULE: fileURLToPath(new URL('./auth-profiles.ts', import.meta.url)),
|
||||
CHATLAB_PROFILE_COUNT: String(count),
|
||||
CHATLAB_WORKER_INDEX: String(workerIndex),
|
||||
...(uniqueBaseName ? { CHATLAB_UNIQUE_BASE_NAME: uniqueBaseName } : {}),
|
||||
HOME: homeDir,
|
||||
},
|
||||
stdio: ['ignore', 'ignore', 'pipe'],
|
||||
@@ -71,3 +82,30 @@ test('auth profile writes preserve updates from concurrent runtime instances', a
|
||||
[]
|
||||
)
|
||||
})
|
||||
|
||||
test('unique auth profile writes allocate distinct names across concurrent runtime instances', async (t) => {
|
||||
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chatlab-auth-profiles-unique-concurrent-'))
|
||||
t.after(() => fs.rmSync(homeDir, { recursive: true, force: true }))
|
||||
|
||||
const workerCount = 4
|
||||
const profilesPerWorker = 8
|
||||
await Promise.all(
|
||||
Array.from({ length: workerCount }, (_, workerIndex) =>
|
||||
runAuthProfileWriterProcess(homeDir, workerIndex, profilesPerWorker, 'shared')
|
||||
)
|
||||
)
|
||||
|
||||
const configDir = path.join(homeDir, '.chatlab')
|
||||
const data = JSON.parse(fs.readFileSync(path.join(configDir, 'auth-profiles.json'), 'utf-8')) as AuthProfilesData
|
||||
const expectedCount = workerCount * profilesPerWorker
|
||||
assert.equal(Object.keys(data.profiles).length, expectedCount)
|
||||
assert.equal(new Set(Object.values(data.profiles).map((profile) => profile.key)).size, expectedCount)
|
||||
assert.ok(data.profiles.shared)
|
||||
assert.ok(data.profiles[`shared-${expectedCount}`])
|
||||
assert.deepEqual(
|
||||
fs
|
||||
.readdirSync(configDir)
|
||||
.filter((name) => name.includes('.lock') || name.startsWith('.chatlab-lock-recovery-') || name.includes('.tmp-')),
|
||||
[]
|
||||
)
|
||||
})
|
||||
@@ -102,6 +102,27 @@ export function writeAuthProfile(name: string, profile: AuthProfile): void {
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* 以 baseName 写入新的 profile;名称分配和写入共用同一把文件锁,避免并发覆盖。
|
||||
* 如果名称已存在,依次尝试 baseName-2、baseName-3……并返回实际名称。
|
||||
*/
|
||||
export function writeAuthProfileWithUniqueName(baseName: string, profile: AuthProfile): string {
|
||||
const filePath = getAuthProfilesPath()
|
||||
return withFileLock(filePath, () => {
|
||||
const data = loadAuthProfiles()
|
||||
let name = baseName
|
||||
let suffix = 2
|
||||
while (Object.hasOwn(data.profiles, name)) {
|
||||
name = `${baseName}-${suffix}`
|
||||
suffix += 1
|
||||
}
|
||||
|
||||
data.profiles = { ...data.profiles, [name]: profile }
|
||||
writeJsonFileAtomically(filePath, data, 0o600)
|
||||
return name
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* 删除一个 auth profile
|
||||
*/
|
||||
|
||||
@@ -15,6 +15,7 @@ export {
|
||||
getApiKeyByProvider,
|
||||
resolveApiKey,
|
||||
writeAuthProfile,
|
||||
writeAuthProfileWithUniqueName,
|
||||
deleteAuthProfile,
|
||||
deriveAuthProfileName,
|
||||
} from './auth-profiles'
|
||||
|
||||
@@ -10,8 +10,15 @@ const tempHome = mkdtempSync(join(tmpdir(), 'chatlab-config-migration-'))
|
||||
const originalHome = process.env.HOME
|
||||
process.env.HOME = tempHome
|
||||
|
||||
const [{ m004EncryptedKeysToAuthProfiles }, { MigrationRunner }, { getApiKeyByProfile, loadAuthProfiles }] =
|
||||
await Promise.all([import('./m004-encrypted-keys-to-auth-profiles'), import('./runner'), import('../auth-profiles')])
|
||||
const [
|
||||
{ m004EncryptedKeysToAuthProfiles },
|
||||
{ MigrationRunner },
|
||||
{ getApiKeyByProfile, loadAuthProfiles, writeAuthProfile },
|
||||
] = await Promise.all([
|
||||
import('./m004-encrypted-keys-to-auth-profiles'),
|
||||
import('./runner'),
|
||||
import('../auth-profiles'),
|
||||
])
|
||||
|
||||
const chatlabDir = join(tempHome, '.chatlab')
|
||||
const aiDataDir = join(chatlabDir, 'ai')
|
||||
@@ -128,6 +135,22 @@ describe('m004 encrypted keys to auth profiles migration', () => {
|
||||
assert.equal(readLlmConfig().configs[0].apiKey, '')
|
||||
})
|
||||
|
||||
it('preserves existing auth profiles and stores migrated keys under unique names', async () => {
|
||||
writeAuthProfile('deepseek', { type: 'api_key', provider: 'deepseek', key: 'existing-primary' })
|
||||
writeAuthProfile('deepseek-2', { type: 'api_key', provider: 'deepseek', key: 'existing-backup' })
|
||||
await writeLlmConfig([{ name: 'Legacy DeepSeek', provider: 'deepseek', apiKey: 'legacy-key' }])
|
||||
|
||||
const { logger } = createLogger()
|
||||
await m004EncryptedKeysToAuthProfiles.up({ dataDir: chatlabDir, aiDataDir, logger })
|
||||
|
||||
const profiles = loadAuthProfiles().profiles
|
||||
assert.equal(profiles.deepseek.key, 'existing-primary')
|
||||
assert.equal(profiles['deepseek-2'].key, 'existing-backup')
|
||||
assert.equal(profiles['deepseek-3'].key, 'legacy-key')
|
||||
assert.equal(readLlmConfig().configs[0].authProfile, 'deepseek-3')
|
||||
assert.equal(readLlmConfig().configs[0].apiKey, '')
|
||||
})
|
||||
|
||||
it('does not create an auth profile or clear apiKey when encrypted key decryption fails', async () => {
|
||||
await writeLlmConfig([{ name: 'Google', provider: 'google', apiKey: 'enc:not:a:valid-key' }])
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
import * as path from 'path'
|
||||
import type { Migration, MigrationContext } from './types'
|
||||
import { isEncrypted, decryptApiKey } from './crypto-legacy'
|
||||
import { deriveAuthProfileName, writeAuthProfile } from '../auth-profiles'
|
||||
import { deriveAuthProfileName, writeAuthProfileWithUniqueName } from '../auth-profiles'
|
||||
import { readJsonFile, withFileLock, writeJsonFileAtomically } from '../atomic-json-file'
|
||||
|
||||
export const m004EncryptedKeysToAuthProfiles: Migration = {
|
||||
@@ -26,7 +26,6 @@ export const m004EncryptedKeysToAuthProfiles: Migration = {
|
||||
|
||||
const configs = (data.configs as Array<Record<string, unknown>>) || []
|
||||
let migrated = false
|
||||
const usedProfileNames = new Set<string>()
|
||||
|
||||
for (const config of configs) {
|
||||
const apiKey = config.apiKey as string
|
||||
@@ -47,16 +46,11 @@ export const m004EncryptedKeysToAuthProfiles: Migration = {
|
||||
|
||||
if (plainKey) {
|
||||
const baseName = deriveAuthProfileName(provider, config)
|
||||
|
||||
let profileName = baseName
|
||||
if (usedProfileNames.has(profileName)) {
|
||||
let i = 2
|
||||
while (usedProfileNames.has(`${profileName}-${i}`)) i++
|
||||
profileName = `${profileName}-${i}`
|
||||
}
|
||||
usedProfileNames.add(profileName)
|
||||
|
||||
writeAuthProfile(profileName, { type: 'api_key', provider, key: plainKey })
|
||||
const profileName = writeAuthProfileWithUniqueName(baseName, {
|
||||
type: 'api_key',
|
||||
provider,
|
||||
key: plainKey,
|
||||
})
|
||||
config.authProfile = profileName
|
||||
config.apiKey = ''
|
||||
migrated = true
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import * as fs from 'node:fs'
|
||||
import * as os from 'node:os'
|
||||
import * as path from 'node:path'
|
||||
import test from 'node:test'
|
||||
import { MigrationRunner } from './runner'
|
||||
import type { Migration } from './types'
|
||||
|
||||
test('migration runner rejects when the migration version marker cannot be written', async (t) => {
|
||||
const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chatlab-migration-marker-failure-'))
|
||||
t.after(() => fs.rmSync(dataDir, { recursive: true, force: true }))
|
||||
fs.mkdirSync(path.join(dataDir, '.migration-version'))
|
||||
|
||||
let migrated = false
|
||||
const migration: Migration = {
|
||||
version: 1,
|
||||
name: 'test-migration',
|
||||
description: 'Test migration marker failure',
|
||||
async up() {
|
||||
migrated = true
|
||||
},
|
||||
}
|
||||
const errors: unknown[][] = []
|
||||
const runner = new MigrationRunner([migration], {
|
||||
dataDir,
|
||||
aiDataDir: path.join(dataDir, 'ai'),
|
||||
logger: {
|
||||
info: () => undefined,
|
||||
warn: () => undefined,
|
||||
error: (...args: unknown[]) => errors.push(args),
|
||||
},
|
||||
})
|
||||
|
||||
await assert.rejects(runner.run(), /migration-version/)
|
||||
assert.equal(migrated, true)
|
||||
assert.equal(errors.length, 1)
|
||||
})
|
||||
@@ -36,14 +36,10 @@ export class MigrationRunner {
|
||||
|
||||
private writeVersion(version: number): void {
|
||||
const versionPath = path.join(this.context.dataDir, VERSION_FILE)
|
||||
try {
|
||||
if (!fs.existsSync(this.context.dataDir)) {
|
||||
fs.mkdirSync(this.context.dataDir, { recursive: true })
|
||||
}
|
||||
fs.writeFileSync(versionPath, String(version), 'utf-8')
|
||||
} catch (err) {
|
||||
this.context.logger.error('Migration', `Failed to write version file: ${err}`)
|
||||
if (!fs.existsSync(this.context.dataDir)) {
|
||||
fs.mkdirSync(this.context.dataDir, { recursive: true })
|
||||
}
|
||||
fs.writeFileSync(versionPath, String(version), 'utf-8')
|
||||
}
|
||||
|
||||
async run(): Promise<{ executed: number; currentVersion: number }> {
|
||||
@@ -63,14 +59,21 @@ export class MigrationRunner {
|
||||
this.context.logger.info('Migration', `Running: v${lastVersion}→v${migration.version} ${migration.name}`)
|
||||
try {
|
||||
await migration.up(this.context)
|
||||
lastVersion = migration.version
|
||||
this.writeVersion(lastVersion)
|
||||
executed++
|
||||
this.context.logger.info('Migration', `Completed: ${migration.name}`)
|
||||
} catch (err) {
|
||||
this.context.logger.error('Migration', `Failed: ${migration.name}`, err)
|
||||
break
|
||||
}
|
||||
|
||||
try {
|
||||
this.writeVersion(migration.version)
|
||||
} catch (err) {
|
||||
this.context.logger.error('Migration', `Failed to persist version ${migration.version}`, err)
|
||||
throw err
|
||||
}
|
||||
|
||||
lastVersion = migration.version
|
||||
executed++
|
||||
this.context.logger.info('Migration', `Completed: ${migration.name}`)
|
||||
}
|
||||
|
||||
return { executed, currentVersion: lastVersion }
|
||||
|
||||
@@ -158,6 +158,48 @@ test('createNodeDataDirSwitch accepts existing CLI data directories without mark
|
||||
assert.equal(result.requiresRelaunch, true)
|
||||
})
|
||||
|
||||
test('createNodeDataDirSwitch rejects unsafe migration targets without creating a pending task', (t) => {
|
||||
const root = makeTempDir()
|
||||
t.after(() => fs.rmSync(root, { recursive: true, force: true }))
|
||||
const currentDir = path.join(root, 'current')
|
||||
writeFile(path.join(currentDir, 'databases', 'current.db'), 'sqlite')
|
||||
|
||||
const unrelatedDir = path.join(root, 'unrelated')
|
||||
writeFile(path.join(unrelatedDir, 'personal.txt'), 'keep me')
|
||||
|
||||
const cases = [
|
||||
{
|
||||
name: 'system directory',
|
||||
targetDir: process.platform === 'win32' ? 'C:\\Windows\\ChatLab' : '/usr/chatlab',
|
||||
expectedError: /System directories/,
|
||||
},
|
||||
{
|
||||
name: 'target nested inside current data directory',
|
||||
targetDir: path.join(currentDir, 'nested'),
|
||||
expectedError: /cannot be inside current data directory/,
|
||||
},
|
||||
{
|
||||
name: 'non-empty unrelated directory',
|
||||
targetDir: unrelatedDir,
|
||||
expectedError: /not empty and is not a ChatLab data directory/,
|
||||
},
|
||||
]
|
||||
|
||||
for (const entry of cases) {
|
||||
const systemDir = path.join(root, `system-${entry.name.replaceAll(' ', '-')}`)
|
||||
const result = createNodeDataDirSwitch({
|
||||
systemDir,
|
||||
currentDir,
|
||||
targetDir: entry.targetDir,
|
||||
migrate: true,
|
||||
})
|
||||
|
||||
assert.equal(result.success, false, entry.name)
|
||||
assert.match(result.error ?? '', entry.expectedError, entry.name)
|
||||
assert.equal(getPendingNodeDataDirMigration(systemDir), null, entry.name)
|
||||
}
|
||||
})
|
||||
|
||||
test('NodePathProvider marks CLI-created data directories', () => {
|
||||
const root = makeTempDir()
|
||||
const dataDir = path.join(root, 'data')
|
||||
@@ -315,6 +357,42 @@ test('manual cleanup refuses directories that are still in use or no longer cont
|
||||
assert.equal(fs.readFileSync(path.join(sourceDir, 'personal.txt'), 'utf-8'), 'keep me')
|
||||
})
|
||||
|
||||
test('manual cleanup refuses source directories that contain or are contained by the system directory', (t) => {
|
||||
const root = makeTempDir()
|
||||
t.after(() => fs.rmSync(root, { recursive: true, force: true }))
|
||||
const cases = [
|
||||
{
|
||||
name: 'source contains system directory',
|
||||
sourceDir: path.join(root, 'parent-source'),
|
||||
systemDir: path.join(root, 'parent-source', 'system'),
|
||||
},
|
||||
{
|
||||
name: 'source is inside system directory',
|
||||
sourceDir: path.join(root, 'system-parent', 'nested-source'),
|
||||
systemDir: path.join(root, 'system-parent'),
|
||||
},
|
||||
]
|
||||
|
||||
for (const entry of cases) {
|
||||
const currentDir = path.join(root, `current-${entry.name.replaceAll(' ', '-')}`)
|
||||
writeFile(path.join(entry.sourceDir, '.chatlab'), 'ChatLab Data Directory')
|
||||
writeFile(path.join(entry.sourceDir, 'databases', 'session.db'), 'sqlite')
|
||||
|
||||
const cleanup = registerPendingDataDirCleanup(entry.systemDir, {
|
||||
sourceDir: entry.sourceDir,
|
||||
targetDir: currentDir,
|
||||
})
|
||||
assert.ok(cleanup, entry.name)
|
||||
|
||||
const result = deletePendingDataDirCleanup(entry.systemDir, currentDir, cleanup.id)
|
||||
|
||||
assert.equal(result.success, false, entry.name)
|
||||
assert.match(result.error ?? '', /overlaps a directory still in use/, entry.name)
|
||||
assert.equal(fs.existsSync(path.join(entry.sourceDir, 'databases', 'session.db')), true, entry.name)
|
||||
assert.equal(getPendingDataDirCleanups(entry.systemDir).length, 1, entry.name)
|
||||
}
|
||||
})
|
||||
|
||||
test('switching back to a preserved directory removes it from cleanup candidates', () => {
|
||||
const root = makeTempDir()
|
||||
const systemDir = path.join(root, 'system')
|
||||
|
||||
@@ -385,7 +385,9 @@ export function deletePendingDataDirCleanup(
|
||||
isSubPath(sourceDir, currentDir) ||
|
||||
isSubPath(currentDir, sourceDir)
|
||||
const overlapsSystemDir =
|
||||
normalizePathForCompare(sourceDir) === normalizePathForCompare(systemDir) || isSubPath(sourceDir, systemDir)
|
||||
normalizePathForCompare(sourceDir) === normalizePathForCompare(systemDir) ||
|
||||
isSubPath(sourceDir, systemDir) ||
|
||||
isSubPath(systemDir, sourceDir)
|
||||
|
||||
if (overlapsCurrentDir || overlapsSystemDir) {
|
||||
appLogger.warn('data-dir', 'Old data directory cleanup rejected because the path is still in use', { cleanupId })
|
||||
|
||||
Reference in new issue
Block a user