From 0015dd88af7e42abbf18ba56262b44339fe1fe46 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 13 Mar 2026 10:55:51 +0900 Subject: [PATCH] fix(agent-config): normalize agent names before builtin override filtering to prevent alias bypass --- .../agent-config-handler.test.ts | 197 ++++++++++++++++++ src/plugin-handlers/agent-config-handler.ts | 47 +++-- .../agent-override-protection.ts | 29 +++ 3 files changed, 256 insertions(+), 17 deletions(-) create mode 100644 src/plugin-handlers/agent-config-handler.test.ts create mode 100644 src/plugin-handlers/agent-override-protection.ts diff --git a/src/plugin-handlers/agent-config-handler.test.ts b/src/plugin-handlers/agent-config-handler.test.ts new file mode 100644 index 000000000..c0d13016c --- /dev/null +++ b/src/plugin-handlers/agent-config-handler.test.ts @@ -0,0 +1,197 @@ +/// + +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 { + 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 + let createSisyphusJuniorAgentSpy: ReturnType + let discoverConfigSourceSkillsSpy: ReturnType + let discoverUserClaudeSkillsSpy: ReturnType + let discoverProjectClaudeSkillsSpy: ReturnType + let discoverOpencodeGlobalSkillsSpy: ReturnType + let discoverOpencodeProjectSkillsSpy: ReturnType + let loadUserAgentsSpy: ReturnType + let loadProjectAgentsSpy: ReturnType + let migrateAgentConfigSpy: ReturnType + let logSpy: ReturnType + + 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) => 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) + }) +}) diff --git a/src/plugin-handlers/agent-config-handler.ts b/src/plugin-handlers/agent-config-handler.ts index ac4c00bcc..4f5ad9eaf 100644 --- a/src/plugin-handlers/agent-config-handler.ts +++ b/src/plugin-handlers/agent-config-handler.ts @@ -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, }; } diff --git a/src/plugin-handlers/agent-override-protection.ts b/src/plugin-handlers/agent-override-protection.ts new file mode 100644 index 000000000..389a54af9 --- /dev/null +++ b/src/plugin-handlers/agent-override-protection.ts @@ -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): Set { + const protectedAgentNames = new Set() + + for (const agentName of agentNames) { + const normalizedAgentName = normalizeProtectedAgentName(agentName) + if (normalizedAgentName.length === 0) continue + + protectedAgentNames.add(normalizedAgentName) + } + + return protectedAgentNames +} + +export function filterProtectedAgentOverrides( + agents: Record, + protectedAgentNames: ReadonlySet, +): Record { + return Object.fromEntries( + Object.entries(agents).filter(([agentName]) => { + return !protectedAgentNames.has(normalizeProtectedAgentName(agentName)) + }), + ) +}