bkinnightskytw
/
agent-orchestrator
mirror of https://github.com/ComposioHQ/agent-orchestrator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

feat(core): enforce reserved fields in notifier configs and improve loader error handling

This commit is contained in:
harshitsinghbhandari 2026-04-02 23:38:13 +05:30 committed by Gaurav Bhola
parent ff47d363c1
commit 4645f6574b
2 changed files with 44 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
packages/core/src/__tests__/plugin-registry.test.ts
View File

@ -262,7 +262,7 @@ describe("loadBuiltins", () => {
});
});
it("strips package and path loading metadata from notifier config", async () => {
it("strips package loading metadata from notifier config", async () => {
const registry = createPluginRegistry();
const fakeWebhook = makePlugin("notifier", "webhook");
const cfg = makeOrchestratorConfig({
@ -270,9 +270,8 @@ describe("loadBuiltins", () => {
notifiers: {
mywebhook: {
plugin: "webhook",
// These are loading metadata fields that should be stripped:
// package field is allowed for resolution but should be stripped:
package: "@composio/ao-plugin-notifier-webhook",
path: "./plugins/custom-webhook", // Filesystem path that could leak
// These are plugin-specific fields that should be passed through:
url: "https://webhook.example.com/notify",
retries: 3,
@ -285,7 +284,7 @@ describe("loadBuiltins", () => {
throw new Error(`Not found: ${pkg}`);
});
// Loading metadata (package, path) should be stripped to prevent leakage
// Loading metadata (package) should be stripped to prevent leakage
// Plugin-specific fields (url, retries) should be passed through
expect(fakeWebhook.create).toHaveBeenCalledWith({
url: "https://webhook.example.com/notify",
@ -294,6 +293,26 @@ describe("loadBuiltins", () => {
});
});
it("throws when path is used alongside plugin name in notifier config", async () => {
const registry = createPluginRegistry();
const fakeWebhook = makePlugin("notifier", "webhook");
const cfg = makeOrchestratorConfig({
notifiers: {
mywebhook: {
plugin: "webhook",
path: "./some/path", // This triggers the collision check
},
},
});
await expect(
registry.loadBuiltins(cfg, async (pkg: string) => {
if (pkg === "@composio/ao-plugin-notifier-webhook") return fakeWebhook;
return null;
})
).rejects.toThrow(/path" is reserved/);
});
it("does not match notifier key when explicit plugin points to another notifier", async () => {
const registry = createPluginRegistry();
const fakeOpenClaw = makePlugin("notifier", "openclaw");

36
packages/core/src/plugin-registry.ts
View File

@ -78,7 +78,8 @@ function extractPluginConfig(
if (matches) {
const rawConfig = notifierConfig as Record<string, unknown>;
// Explicitly check for reserved fields to prevent silent stripping/collision
// Explicitly check for reserved fields to prevent silent stripping/collision.
// 'path' is reserved for local resolution; 'package' is reserved for npm resolution.
if ("package" in rawConfig && "path" in rawConfig) {
throw new Error(
`In notifier "${notifierName}": both "package" and "path" are specified. ` +
@ -86,13 +87,15 @@ function extractPluginConfig(
);
}
if (hasExplicitPlugin || rawConfig.package) {
if ("path" in rawConfig) {
throw new Error(
`In notifier "${notifierName}": "path" is reserved for plugin loading. ` +
`Rename your configuration field to something else (e.g., "apiPath").`
);
}
// If loading via built-in name or npm package, 'path' is a collision.
// We detect this by checking if 'package' is present OR if no 'path' was
// intended for local resolution (i.e. name refers to a built-in).
const isBuiltin = BUILTIN_PLUGINS.some((b) => b.slot === slot && b.name === name);
if ((rawConfig.package || isBuiltin) && "path" in rawConfig) {
throw new Error(
`In notifier "${notifierName}": "path" is reserved for plugin loading. ` +
`Rename your configuration field to something else (e.g., "apiPath").`
);
}
// Strip loading metadata fields (plugin, package, path) from config passed to plugin.
@ -344,16 +347,19 @@ export function createPluginRegistry(): PluginRegistry {
): Promise<void> {
const doImport = importFn ?? ((pkg: string) => import(pkg));
for (const builtin of BUILTIN_PLUGINS) {
let mod;
try {
const mod = normalizeImportedPluginModule(await doImport(builtin.pkg));
if (mod) {
const pluginConfig = orchestratorConfig
? extractPluginConfig(builtin.slot, builtin.name, orchestratorConfig)
: undefined;
this.register(mod, pluginConfig);
}
mod = normalizeImportedPluginModule(await doImport(builtin.pkg));
} catch {
// Plugin not installed — that's fine, only load what's available
continue;
}
if (mod) {
const pluginConfig = orchestratorConfig
? extractPluginConfig(builtin.slot, builtin.name, orchestratorConfig)
: undefined;
this.register(mod, pluginConfig);
}
}
},