fix(agent-config): normalize agent names before builtin override filtering to prevent alias bypass
This commit is contained in:
197
src/plugin-handlers/agent-config-handler.test.ts
Normal file
197
src/plugin-handlers/agent-config-handler.test.ts
Normal file
@@ -0,0 +1,197 @@
|
||||
/// <reference types="bun-types" />
|
||||
|
||||
import type { AgentConfig } from "@opencode-ai/sdk"
|
||||
import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"
|
||||
import * as agents from "../agents"
|
||||
import * as shared from "../shared"
|
||||
import * as sisyphusJunior from "../agents/sisyphus-junior"
|
||||
import type { OhMyOpenCodeConfig } from "../config"
|
||||
import * as agentLoader from "../features/claude-code-agent-loader"
|
||||
import * as skillLoader from "../features/opencode-skill-loader"
|
||||
import { getAgentDisplayName } from "../shared/agent-display-names"
|
||||
import { applyAgentConfig } from "./agent-config-handler"
|
||||
import type { PluginComponents } from "./plugin-components-loader"
|
||||
|
||||
const BUILTIN_SISYPHUS_DISPLAY_NAME = getAgentDisplayName("sisyphus")
|
||||
|
||||
function createPluginComponents(): PluginComponents {
|
||||
return {
|
||||
commands: {},
|
||||
skills: {},
|
||||
agents: {},
|
||||
mcpServers: {},
|
||||
hooksConfigs: [],
|
||||
plugins: [],
|
||||
errors: [],
|
||||
}
|
||||
}
|
||||
|
||||
function createBaseConfig(): Record<string, unknown> {
|
||||
return {
|
||||
model: "anthropic/claude-opus-4-6",
|
||||
agent: {},
|
||||
}
|
||||
}
|
||||
|
||||
function createPluginConfig(): OhMyOpenCodeConfig {
|
||||
return {
|
||||
sisyphus_agent: {
|
||||
planner_enabled: false,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
describe("applyAgentConfig builtin override protection", () => {
|
||||
let createBuiltinAgentsSpy: ReturnType<typeof spyOn>
|
||||
let createSisyphusJuniorAgentSpy: ReturnType<typeof spyOn>
|
||||
let discoverConfigSourceSkillsSpy: ReturnType<typeof spyOn>
|
||||
let discoverUserClaudeSkillsSpy: ReturnType<typeof spyOn>
|
||||
let discoverProjectClaudeSkillsSpy: ReturnType<typeof spyOn>
|
||||
let discoverOpencodeGlobalSkillsSpy: ReturnType<typeof spyOn>
|
||||
let discoverOpencodeProjectSkillsSpy: ReturnType<typeof spyOn>
|
||||
let loadUserAgentsSpy: ReturnType<typeof spyOn>
|
||||
let loadProjectAgentsSpy: ReturnType<typeof spyOn>
|
||||
let migrateAgentConfigSpy: ReturnType<typeof spyOn>
|
||||
let logSpy: ReturnType<typeof spyOn>
|
||||
|
||||
const builtinSisyphusConfig: AgentConfig = {
|
||||
name: "Builtin Sisyphus",
|
||||
prompt: "builtin prompt",
|
||||
mode: "primary",
|
||||
}
|
||||
|
||||
const builtinOracleConfig: AgentConfig = {
|
||||
name: "oracle",
|
||||
prompt: "oracle prompt",
|
||||
mode: "subagent",
|
||||
}
|
||||
|
||||
const sisyphusJuniorConfig: AgentConfig = {
|
||||
name: "Sisyphus-Junior",
|
||||
prompt: "junior prompt",
|
||||
mode: "all",
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
createBuiltinAgentsSpy = spyOn(agents, "createBuiltinAgents").mockResolvedValue({
|
||||
sisyphus: builtinSisyphusConfig,
|
||||
oracle: builtinOracleConfig,
|
||||
})
|
||||
|
||||
createSisyphusJuniorAgentSpy = spyOn(
|
||||
sisyphusJunior,
|
||||
"createSisyphusJuniorAgentWithOverrides",
|
||||
).mockReturnValue(sisyphusJuniorConfig)
|
||||
|
||||
discoverConfigSourceSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverConfigSourceSkills",
|
||||
).mockResolvedValue([])
|
||||
discoverUserClaudeSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverUserClaudeSkills",
|
||||
).mockResolvedValue([])
|
||||
discoverProjectClaudeSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverProjectClaudeSkills",
|
||||
).mockResolvedValue([])
|
||||
discoverOpencodeGlobalSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverOpencodeGlobalSkills",
|
||||
).mockResolvedValue([])
|
||||
discoverOpencodeProjectSkillsSpy = spyOn(
|
||||
skillLoader,
|
||||
"discoverOpencodeProjectSkills",
|
||||
).mockResolvedValue([])
|
||||
|
||||
loadUserAgentsSpy = spyOn(agentLoader, "loadUserAgents").mockReturnValue({})
|
||||
loadProjectAgentsSpy = spyOn(agentLoader, "loadProjectAgents").mockReturnValue({})
|
||||
|
||||
migrateAgentConfigSpy = spyOn(shared, "migrateAgentConfig").mockImplementation(
|
||||
(config: Record<string, unknown>) => config,
|
||||
)
|
||||
logSpy = spyOn(shared, "log").mockImplementation(() => {})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
createBuiltinAgentsSpy.mockRestore()
|
||||
createSisyphusJuniorAgentSpy.mockRestore()
|
||||
discoverConfigSourceSkillsSpy.mockRestore()
|
||||
discoverUserClaudeSkillsSpy.mockRestore()
|
||||
discoverProjectClaudeSkillsSpy.mockRestore()
|
||||
discoverOpencodeGlobalSkillsSpy.mockRestore()
|
||||
discoverOpencodeProjectSkillsSpy.mockRestore()
|
||||
loadUserAgentsSpy.mockRestore()
|
||||
loadProjectAgentsSpy.mockRestore()
|
||||
migrateAgentConfigSpy.mockRestore()
|
||||
logSpy.mockRestore()
|
||||
})
|
||||
|
||||
test("filters user agents whose key matches the builtin display-name alias", async () => {
|
||||
// given
|
||||
loadUserAgentsSpy.mockReturnValue({
|
||||
[BUILTIN_SISYPHUS_DISPLAY_NAME]: {
|
||||
name: BUILTIN_SISYPHUS_DISPLAY_NAME,
|
||||
prompt: "user alias prompt",
|
||||
mode: "subagent",
|
||||
},
|
||||
})
|
||||
|
||||
// when
|
||||
const result = await applyAgentConfig({
|
||||
config: createBaseConfig(),
|
||||
pluginConfig: createPluginConfig(),
|
||||
ctx: { directory: "/tmp" },
|
||||
pluginComponents: createPluginComponents(),
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result[BUILTIN_SISYPHUS_DISPLAY_NAME]).toEqual(builtinSisyphusConfig)
|
||||
})
|
||||
|
||||
test("filters user agents whose key differs from a builtin key only by case", async () => {
|
||||
// given
|
||||
loadUserAgentsSpy.mockReturnValue({
|
||||
SiSyPhUs: {
|
||||
name: "SiSyPhUs",
|
||||
prompt: "mixed-case prompt",
|
||||
mode: "subagent",
|
||||
},
|
||||
})
|
||||
|
||||
// when
|
||||
const result = await applyAgentConfig({
|
||||
config: createBaseConfig(),
|
||||
pluginConfig: createPluginConfig(),
|
||||
ctx: { directory: "/tmp" },
|
||||
pluginComponents: createPluginComponents(),
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result[BUILTIN_SISYPHUS_DISPLAY_NAME]).toEqual(builtinSisyphusConfig)
|
||||
expect(result.SiSyPhUs).toBeUndefined()
|
||||
})
|
||||
|
||||
test("filters plugin agents whose key matches the builtin display-name alias", async () => {
|
||||
// given
|
||||
const pluginComponents = createPluginComponents()
|
||||
pluginComponents.agents = {
|
||||
[BUILTIN_SISYPHUS_DISPLAY_NAME]: {
|
||||
name: BUILTIN_SISYPHUS_DISPLAY_NAME,
|
||||
prompt: "plugin alias prompt",
|
||||
mode: "subagent",
|
||||
},
|
||||
}
|
||||
|
||||
// when
|
||||
const result = await applyAgentConfig({
|
||||
config: createBaseConfig(),
|
||||
pluginConfig: createPluginConfig(),
|
||||
ctx: { directory: "/tmp" },
|
||||
pluginComponents,
|
||||
})
|
||||
|
||||
// then
|
||||
expect(result[BUILTIN_SISYPHUS_DISPLAY_NAME]).toEqual(builtinSisyphusConfig)
|
||||
})
|
||||
})
|
||||
@@ -15,6 +15,10 @@ import { loadProjectAgents, loadUserAgents } from "../features/claude-code-agent
|
||||
import type { PluginComponents } from "./plugin-components-loader";
|
||||
import { reorderAgentsByPriority } from "./agent-priority-order";
|
||||
import { remapAgentKeysToDisplayNames } from "./agent-key-remapper";
|
||||
import {
|
||||
createProtectedAgentNameSet,
|
||||
filterProtectedAgentOverrides,
|
||||
} from "./agent-override-protection";
|
||||
import { buildPrometheusAgentConfig } from "./prometheus-agent-config-builder";
|
||||
import { buildPlanDemoteConfig } from "./plan-model-inheritance";
|
||||
|
||||
@@ -209,19 +213,21 @@ export async function applyAgentConfig(params: {
|
||||
)
|
||||
: undefined;
|
||||
|
||||
// Collect all builtin agent names to prevent user/project .md files from overriding them
|
||||
const builtinAgentNames = new Set([
|
||||
const protectedBuiltinAgentNames = createProtectedAgentNameSet([
|
||||
...Object.keys(agentConfig),
|
||||
...Object.keys(builtinAgents),
|
||||
]);
|
||||
|
||||
// Filter user/project agents that duplicate builtin agents (they have mode: "subagent" hardcoded
|
||||
// in loadAgentsFromDir which would incorrectly override the builtin mode: "primary")
|
||||
const filteredUserAgents = Object.fromEntries(
|
||||
Object.entries(userAgents).filter(([key]) => !builtinAgentNames.has(key)),
|
||||
const filteredUserAgents = filterProtectedAgentOverrides(
|
||||
userAgents,
|
||||
protectedBuiltinAgentNames,
|
||||
);
|
||||
const filteredProjectAgents = Object.fromEntries(
|
||||
Object.entries(projectAgents).filter(([key]) => !builtinAgentNames.has(key)),
|
||||
const filteredProjectAgents = filterProtectedAgentOverrides(
|
||||
projectAgents,
|
||||
protectedBuiltinAgentNames,
|
||||
);
|
||||
const filteredPluginAgents = filterProtectedAgentOverrides(
|
||||
pluginAgents,
|
||||
protectedBuiltinAgentNames,
|
||||
);
|
||||
|
||||
params.config.agent = {
|
||||
@@ -231,26 +237,33 @@ export async function applyAgentConfig(params: {
|
||||
),
|
||||
...filterDisabledAgents(filteredUserAgents),
|
||||
...filterDisabledAgents(filteredProjectAgents),
|
||||
...filterDisabledAgents(pluginAgents),
|
||||
...filterDisabledAgents(filteredPluginAgents),
|
||||
...filteredConfigAgents,
|
||||
build: { ...migratedBuild, mode: "subagent", hidden: true },
|
||||
...(planDemoteConfig ? { plan: planDemoteConfig } : {}),
|
||||
};
|
||||
} else {
|
||||
// Filter user/project agents that duplicate builtin agents
|
||||
const builtinAgentNames = new Set(Object.keys(builtinAgents));
|
||||
const filteredUserAgents = Object.fromEntries(
|
||||
Object.entries(userAgents).filter(([key]) => !builtinAgentNames.has(key)),
|
||||
const protectedBuiltinAgentNames = createProtectedAgentNameSet(
|
||||
Object.keys(builtinAgents),
|
||||
);
|
||||
const filteredProjectAgents = Object.fromEntries(
|
||||
Object.entries(projectAgents).filter(([key]) => !builtinAgentNames.has(key)),
|
||||
const filteredUserAgents = filterProtectedAgentOverrides(
|
||||
userAgents,
|
||||
protectedBuiltinAgentNames,
|
||||
);
|
||||
const filteredProjectAgents = filterProtectedAgentOverrides(
|
||||
projectAgents,
|
||||
protectedBuiltinAgentNames,
|
||||
);
|
||||
const filteredPluginAgents = filterProtectedAgentOverrides(
|
||||
pluginAgents,
|
||||
protectedBuiltinAgentNames,
|
||||
);
|
||||
|
||||
params.config.agent = {
|
||||
...builtinAgents,
|
||||
...filterDisabledAgents(filteredUserAgents),
|
||||
...filterDisabledAgents(filteredProjectAgents),
|
||||
...filterDisabledAgents(pluginAgents),
|
||||
...filterDisabledAgents(filteredPluginAgents),
|
||||
...configAgent,
|
||||
};
|
||||
}
|
||||
|
||||
29
src/plugin-handlers/agent-override-protection.ts
Normal file
29
src/plugin-handlers/agent-override-protection.ts
Normal file
@@ -0,0 +1,29 @@
|
||||
const PARENTHETICAL_SUFFIX_PATTERN = /\s*(\([^)]*\)\s*)+$/u
|
||||
|
||||
export function normalizeProtectedAgentName(agentName: string): string {
|
||||
return agentName.trim().toLowerCase().replace(PARENTHETICAL_SUFFIX_PATTERN, "").trim()
|
||||
}
|
||||
|
||||
export function createProtectedAgentNameSet(agentNames: Iterable<string>): Set<string> {
|
||||
const protectedAgentNames = new Set<string>()
|
||||
|
||||
for (const agentName of agentNames) {
|
||||
const normalizedAgentName = normalizeProtectedAgentName(agentName)
|
||||
if (normalizedAgentName.length === 0) continue
|
||||
|
||||
protectedAgentNames.add(normalizedAgentName)
|
||||
}
|
||||
|
||||
return protectedAgentNames
|
||||
}
|
||||
|
||||
export function filterProtectedAgentOverrides<TAgent>(
|
||||
agents: Record<string, TAgent>,
|
||||
protectedAgentNames: ReadonlySet<string>,
|
||||
): Record<string, TAgent> {
|
||||
return Object.fromEntries(
|
||||
Object.entries(agents).filter(([agentName]) => {
|
||||
return !protectedAgentNames.has(normalizeProtectedAgentName(agentName))
|
||||
}),
|
||||
)
|
||||
}
|
||||
Reference in New Issue
Block a user