Fix bridge profile environment isolation (#796)
This commit is contained in:
ekko
@@ -0,0 +1,192 @@
|
||||
import { execFile } from 'child_process'
|
||||
import { mkdir, mkdtemp, realpath, rm, writeFile } from 'fs/promises'
|
||||
import { tmpdir } from 'os'
|
||||
import { join, resolve } from 'path'
|
||||
import { promisify } from 'util'
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
const execFileAsync = promisify(execFile)
|
||||
|
||||
let tempDir = ''
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = await mkdtemp(join(tmpdir(), 'hermes-bridge-profile-env-'))
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
if (tempDir) await rm(tempDir, { recursive: true, force: true })
|
||||
tempDir = ''
|
||||
})
|
||||
|
||||
async function runBridgeProbe(script: string): Promise<any> {
|
||||
const bridgePath = resolve('packages/server/src/services/hermes/agent-bridge/hermes_bridge.py')
|
||||
const { stdout } = await execFileAsync('python3', ['-c', script], {
|
||||
cwd: resolve('.'),
|
||||
env: {
|
||||
...process.env,
|
||||
BRIDGE_PATH: bridgePath,
|
||||
TEST_HERMES_HOME: tempDir,
|
||||
},
|
||||
maxBuffer: 1024 * 1024,
|
||||
})
|
||||
return JSON.parse(stdout)
|
||||
}
|
||||
|
||||
describe('agent bridge profile environment', () => {
|
||||
it('runs agent calls with the requested profile HERMES_HOME and restores the bridge home', async () => {
|
||||
const profileHome = join(tempDir, 'profiles', 'work')
|
||||
await mkdir(profileHome, { recursive: true })
|
||||
await writeFile(join(tempDir, 'config.yaml'), 'model:\n default: default-model\n', 'utf-8')
|
||||
await writeFile(join(tempDir, '.env'), 'OPENAI_API_KEY=default-openai\nBASE_ONLY_TOKEN=base-token\n', 'utf-8')
|
||||
await writeFile(join(profileHome, 'config.yaml'), 'model:\n default: work-model\n', 'utf-8')
|
||||
await writeFile(join(profileHome, '.env'), 'GLM_API_KEY=work-glm\n', 'utf-8')
|
||||
const expectedProfileHome = await realpath(profileHome)
|
||||
|
||||
const result = await runBridgeProbe(`
|
||||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
|
||||
spec = importlib.util.spec_from_file_location("hermes_bridge", os.environ["BRIDGE_PATH"])
|
||||
bridge = importlib.util.module_from_spec(spec)
|
||||
sys.modules["hermes_bridge"] = bridge
|
||||
spec.loader.exec_module(bridge)
|
||||
|
||||
root = os.environ["TEST_HERMES_HOME"]
|
||||
profile_home = os.path.join(root, "profiles", "work")
|
||||
os.environ["HERMES_HOME"] = root
|
||||
os.environ["HERMES_AGENT_BRIDGE_BASE_HOME"] = root
|
||||
os.environ["OPENAI_API_KEY"] = "shell-openai"
|
||||
os.environ["GLM_API_KEY"] = "shell-glm"
|
||||
|
||||
class FakeAgent:
|
||||
def __init__(self):
|
||||
self.seen_home = None
|
||||
self.seen_openai = None
|
||||
self.seen_glm = None
|
||||
self.seen_base_only = None
|
||||
|
||||
def run_conversation(self, message, **kwargs):
|
||||
self.seen_home = os.environ.get("HERMES_HOME")
|
||||
self.seen_openai = os.environ.get("OPENAI_API_KEY")
|
||||
self.seen_glm = os.environ.get("GLM_API_KEY")
|
||||
self.seen_base_only = os.environ.get("BASE_ONLY_TOKEN")
|
||||
return {"messages": [{"role": "assistant", "content": "ok"}]}
|
||||
|
||||
agent = FakeAgent()
|
||||
with bridge._profile_env("work"):
|
||||
result = agent.run_conversation("hello")
|
||||
|
||||
print(json.dumps({
|
||||
"seen_home": agent.seen_home,
|
||||
"seen_openai": agent.seen_openai,
|
||||
"seen_glm": agent.seen_glm,
|
||||
"seen_base_only": agent.seen_base_only,
|
||||
"restored_home": os.environ.get("HERMES_HOME"),
|
||||
"restored_openai": os.environ.get("OPENAI_API_KEY"),
|
||||
"restored_glm": os.environ.get("GLM_API_KEY"),
|
||||
"restored_base_only": os.environ.get("BASE_ONLY_TOKEN"),
|
||||
"status": "complete" if result.get("messages") else "error",
|
||||
}))
|
||||
`)
|
||||
|
||||
expect(result).toEqual({
|
||||
seen_home: expectedProfileHome,
|
||||
seen_openai: null,
|
||||
seen_glm: 'work-glm',
|
||||
seen_base_only: null,
|
||||
restored_home: tempDir,
|
||||
restored_openai: 'shell-openai',
|
||||
restored_glm: 'shell-glm',
|
||||
restored_base_only: null,
|
||||
status: 'complete',
|
||||
})
|
||||
})
|
||||
|
||||
it('normalizes a profile-scoped bridge home back to the Hermes root for profile lookup', async () => {
|
||||
const agentRoot = join(tempDir, 'hermes-agent')
|
||||
const profileHome = join(tempDir, 'profiles', 'work')
|
||||
await mkdir(agentRoot, { recursive: true })
|
||||
await mkdir(profileHome, { recursive: true })
|
||||
await writeFile(join(agentRoot, 'run_agent.py'), '', 'utf-8')
|
||||
await writeFile(join(profileHome, 'config.yaml'), 'model:\n default: work-model\n', 'utf-8')
|
||||
const expectedRoot = await realpath(tempDir)
|
||||
const expectedProfileHome = await realpath(profileHome)
|
||||
|
||||
const result = await runBridgeProbe(`
|
||||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
|
||||
spec = importlib.util.spec_from_file_location("hermes_bridge", os.environ["BRIDGE_PATH"])
|
||||
bridge = importlib.util.module_from_spec(spec)
|
||||
sys.modules["hermes_bridge"] = bridge
|
||||
spec.loader.exec_module(bridge)
|
||||
|
||||
root = os.environ["TEST_HERMES_HOME"]
|
||||
agent_root = os.path.join(root, "hermes-agent")
|
||||
profile_home = os.path.join(root, "profiles", "work")
|
||||
bridge._set_path_env(agent_root, profile_home)
|
||||
|
||||
print(json.dumps({
|
||||
"home": os.environ.get("HERMES_HOME"),
|
||||
"base": os.environ.get("HERMES_AGENT_BRIDGE_BASE_HOME"),
|
||||
"profile_home": str(bridge._profile_home("work")),
|
||||
}))
|
||||
`)
|
||||
|
||||
expect(result).toEqual({
|
||||
home: expectedProfileHome,
|
||||
base: expectedRoot,
|
||||
profile_home: expectedProfileHome,
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps inherited profile env keys for default profile compatibility', async () => {
|
||||
await mkdir(join(tempDir, 'profiles', 'work'), { recursive: true })
|
||||
await writeFile(join(tempDir, '.env'), 'OPENAI_API_KEY=default-openai\n', 'utf-8')
|
||||
await writeFile(join(tempDir, 'profiles', 'work', '.env'), 'GLM_API_KEY=work-glm\n', 'utf-8')
|
||||
await writeFile(join(tempDir, 'config.yaml'), 'model:\n default: default-model\n', 'utf-8')
|
||||
|
||||
const result = await runBridgeProbe(`
|
||||
import importlib.util
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
|
||||
spec = importlib.util.spec_from_file_location("hermes_bridge", os.environ["BRIDGE_PATH"])
|
||||
bridge = importlib.util.module_from_spec(spec)
|
||||
sys.modules["hermes_bridge"] = bridge
|
||||
spec.loader.exec_module(bridge)
|
||||
|
||||
root = os.environ["TEST_HERMES_HOME"]
|
||||
os.environ["HERMES_HOME"] = root
|
||||
os.environ["HERMES_AGENT_BRIDGE_BASE_HOME"] = root
|
||||
os.environ["OPENAI_API_KEY"] = "shell-openai"
|
||||
os.environ["GLM_API_KEY"] = "shell-glm"
|
||||
|
||||
with bridge._profile_env("default"):
|
||||
inside = {
|
||||
"openai": os.environ.get("OPENAI_API_KEY"),
|
||||
"glm": os.environ.get("GLM_API_KEY"),
|
||||
}
|
||||
|
||||
print(json.dumps({
|
||||
"inside": inside,
|
||||
"restored_openai": os.environ.get("OPENAI_API_KEY"),
|
||||
"restored_glm": os.environ.get("GLM_API_KEY"),
|
||||
}))
|
||||
`)
|
||||
|
||||
expect(result).toEqual({
|
||||
inside: {
|
||||
openai: 'default-openai',
|
||||
glm: 'shell-glm',
|
||||
},
|
||||
restored_openai: 'shell-openai',
|
||||
restored_glm: 'shell-glm',
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,4 +1,8 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { existsSync, readFileSync } from 'fs'
|
||||
import { mkdir, mkdtemp, rm, writeFile } from 'fs/promises'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
|
||||
|
||||
// Mock hermes-cli
|
||||
vi.mock('../../packages/server/src/services/hermes/hermes-cli', () => ({
|
||||
@@ -19,27 +23,17 @@ vi.mock('../../packages/server/src/services/hermes/hermes-cli', () => ({
|
||||
import * as hermesCli from '../../packages/server/src/services/hermes/hermes-cli'
|
||||
|
||||
describe('Profile Routes', () => {
|
||||
const originalHermesHome = process.env.HERMES_HOME
|
||||
const tempHomes: string[] = []
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
describe('ensureApiServerConfig (via active profile switch)', () => {
|
||||
it('should inject api_server config when missing', async () => {
|
||||
// This tests the logic that profiles.ts ensures api_server config exists
|
||||
// We test the ensureApiServerConfig behavior indirectly through the module
|
||||
const { existsSync, readFileSync, writeFileSync } = await import('fs')
|
||||
vi.mock('fs', () => ({
|
||||
existsSync: vi.fn().mockReturnValue(true),
|
||||
readFileSync: vi.fn().mockReturnValue('platforms: {}'),
|
||||
writeFileSync: vi.fn(),
|
||||
createReadStream: vi.fn(),
|
||||
unlinkSync: vi.fn(),
|
||||
mkdirSync: vi.fn(),
|
||||
copyFileSync: vi.fn(),
|
||||
mkdir: vi.fn(),
|
||||
writeFile: vi.fn(),
|
||||
}))
|
||||
})
|
||||
afterEach(async () => {
|
||||
if (originalHermesHome === undefined) delete process.env.HERMES_HOME
|
||||
else process.env.HERMES_HOME = originalHermesHome
|
||||
await Promise.all(tempHomes.splice(0).map(dir => rm(dir, { recursive: true, force: true })))
|
||||
})
|
||||
|
||||
describe('hermes-cli wrapper', () => {
|
||||
@@ -84,4 +78,43 @@ describe('Profile Routes', () => {
|
||||
expect(hermesCli.renameProfile).toHaveBeenCalledWith('old', 'new')
|
||||
})
|
||||
})
|
||||
|
||||
describe('profile deletion fallback', () => {
|
||||
it('removes a reserved profile directory when Hermes CLI refuses to delete it', async () => {
|
||||
const hermesHome = await mkdtemp(join(tmpdir(), 'hermes-profile-delete-'))
|
||||
tempHomes.push(hermesHome)
|
||||
process.env.HERMES_HOME = hermesHome
|
||||
const badProfileDir = join(hermesHome, 'profiles', 'hermes')
|
||||
await mkdir(badProfileDir, { recursive: true })
|
||||
await writeFile(join(badProfileDir, 'config.yaml'), 'model:\n default: bad\n', 'utf-8')
|
||||
await writeFile(join(hermesHome, 'active_profile'), 'hermes\n', 'utf-8')
|
||||
vi.mocked(hermesCli.deleteProfile).mockResolvedValue(false)
|
||||
const { remove } = await import('../../packages/server/src/controllers/hermes/profiles')
|
||||
const ctx: any = { params: { name: 'hermes' }, status: 200, body: undefined }
|
||||
|
||||
await remove(ctx)
|
||||
|
||||
expect(ctx.status).toBe(200)
|
||||
expect(ctx.body).toEqual({ success: true, fallback: 'removed_reserved_profile_from_disk' })
|
||||
expect(existsSync(badProfileDir)).toBe(false)
|
||||
expect(readFileSync(join(hermesHome, 'active_profile'), 'utf-8')).toBe('default\n')
|
||||
})
|
||||
|
||||
it('does not bypass Hermes CLI failures for normal profile names', async () => {
|
||||
const hermesHome = await mkdtemp(join(tmpdir(), 'hermes-profile-delete-'))
|
||||
tempHomes.push(hermesHome)
|
||||
process.env.HERMES_HOME = hermesHome
|
||||
const profileDir = join(hermesHome, 'profiles', 'work')
|
||||
await mkdir(profileDir, { recursive: true })
|
||||
vi.mocked(hermesCli.deleteProfile).mockResolvedValue(false)
|
||||
const { remove } = await import('../../packages/server/src/controllers/hermes/profiles')
|
||||
const ctx: any = { params: { name: 'work' }, status: 200, body: undefined }
|
||||
|
||||
await remove(ctx)
|
||||
|
||||
expect(ctx.status).toBe(500)
|
||||
expect(ctx.body).toEqual({ error: 'Failed to delete profile' })
|
||||
expect(existsSync(profileDir)).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user