mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-11 00:33:40 +00:00
🪝 feat: Config-driven tool-approval hook loader (librechat.yaml → hook modules)
Lets operators declare programmatic tool-approval hooks in config instead of code, so the
registerToolApprovalHook seam is usable without a custom build.
- Config (data-provider): `endpoints.agents.toolApproval.hooks[]`, each entry
`{ module, matcher?, options? }`. `module` is a bare package name or a path (resolved
against the app root); its default export is a builder `(options?) => ToolApprovalHookFactory`.
- Loader (@librechat/api `loadToolApprovalHooks`): imports each module, builds the factory
with the entry's options, and registers it (with its optional tool-name matcher). Reload-
safe (each call first unregisters its previous batch, leaving code-registered hooks alone)
and robust — an unimportable module / non-function export / throwing builder is logged and
skipped, never crashing startup or blocking the other hooks. Importer is injectable for tests.
- Startup (api/server/index.js): loads the configured hooks once after appConfig resolves.
SECURITY: modules are dynamically imported + executed in-process; this is admin-level config,
documented as trusted-code-only.
Tests: 9 loader cases (default/no-default export, options passthrough, bad-export skip,
builder-returns-non-function skip, import-failure resilience, continue-past-bad-entry,
reload de-dup). Full HITL suite green (80).
This commit is contained in:
parent
b853fc24d9
commit
7f91a8a2aa
5 changed files with 240 additions and 0 deletions
|
|
@ -23,6 +23,7 @@ const {
|
|||
createStreamServices,
|
||||
initializeFileStorage,
|
||||
initializeDeploymentSkills,
|
||||
loadToolApprovalHooks,
|
||||
maybeInjectQueryDevtoolsBootstrap,
|
||||
preAuthTenantMiddleware,
|
||||
setupGracefulShutdown,
|
||||
|
|
@ -124,6 +125,11 @@ const startServer = async () => {
|
|||
await initializeDeploymentSkills({ projectRoot: path.resolve(__dirname, '../..') });
|
||||
initializeGitHubSkillSync(appConfig);
|
||||
startExpiredFileSweep({ appConfig, loadAppConfig: getAppConfig });
|
||||
// Register any programmatic tool-approval policy hooks declared in
|
||||
// `endpoints.agents.toolApproval.hooks` (no-op when none / HITL disabled).
|
||||
await loadToolApprovalHooks(appConfig?.endpoints?.agents?.toolApproval?.hooks, {
|
||||
basePath: path.resolve(__dirname, '../..'),
|
||||
});
|
||||
await runAsSystem(async () => {
|
||||
await performStartupChecks(appConfig);
|
||||
await updateInterfacePermissions({ appConfig, getRoleByName, updateAccessPermissions });
|
||||
|
|
|
|||
92
packages/api/src/agents/hitl/hookLoader.spec.ts
Normal file
92
packages/api/src/agents/hitl/hookLoader.spec.ts
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
import { logger } from '@librechat/data-schemas';
|
||||
import { loadToolApprovalHooks } from './hookLoader';
|
||||
import { getRegisteredToolApprovalHookCount, clearToolApprovalHooks } from './hooks';
|
||||
|
||||
/** A conforming hook module: (options) => (context) => (input) => decision. */
|
||||
const goodModule = (options?: Record<string, unknown>) => () => async () => ({
|
||||
decision: (options?.decision as 'ask') ?? 'ask',
|
||||
});
|
||||
|
||||
describe('loadToolApprovalHooks', () => {
|
||||
beforeEach(() => {
|
||||
jest.spyOn(logger, 'error').mockImplementation(() => logger);
|
||||
jest.spyOn(logger, 'info').mockImplementation(() => logger);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
clearToolApprovalHooks();
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
test('registers a hook from a module default export', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: goodModule }));
|
||||
const n = await loadToolApprovalHooks([{ module: './hook.js', matcher: 'write_.*' }], {
|
||||
importModule,
|
||||
});
|
||||
expect(n).toBe(1);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(1);
|
||||
expect(importModule).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('supports a module that IS the builder (no default export)', async () => {
|
||||
const importModule = jest.fn(async () => goodModule);
|
||||
expect(await loadToolApprovalHooks([{ module: 'some-pkg' }], { importModule })).toBe(1);
|
||||
});
|
||||
|
||||
test('passes the entry options to the builder', async () => {
|
||||
const builder = jest.fn(goodModule);
|
||||
const importModule = jest.fn(async () => ({ default: builder }));
|
||||
await loadToolApprovalHooks([{ module: './h.js', options: { foo: 'bar' } }], { importModule });
|
||||
expect(builder).toHaveBeenCalledWith({ foo: 'bar' });
|
||||
});
|
||||
|
||||
test('returns 0 for empty / undefined config and registers nothing', async () => {
|
||||
expect(await loadToolApprovalHooks(undefined)).toBe(0);
|
||||
expect(await loadToolApprovalHooks([])).toBe(0);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(0);
|
||||
});
|
||||
|
||||
test('skips a module whose export is not a function (no crash)', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: { notAFunction: true } }));
|
||||
expect(await loadToolApprovalHooks([{ module: './bad.js' }], { importModule })).toBe(0);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(0);
|
||||
expect(logger.error).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('skips when the builder returns a non-function', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: () => 'not a factory' }));
|
||||
expect(await loadToolApprovalHooks([{ module: './bad.js' }], { importModule })).toBe(0);
|
||||
});
|
||||
|
||||
test('resolves (does not throw) when a module import fails', async () => {
|
||||
const importModule = jest.fn(async () => {
|
||||
throw new Error('cannot find module');
|
||||
});
|
||||
await expect(
|
||||
loadToolApprovalHooks([{ module: './missing.js' }], { importModule }),
|
||||
).resolves.toBe(0);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(0);
|
||||
expect(logger.error).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('continues past a bad entry to load the good ones', async () => {
|
||||
const importModule = jest.fn(async (spec: string) =>
|
||||
spec.includes('bad') ? Promise.reject(new Error('nope')) : { default: goodModule },
|
||||
);
|
||||
const n = await loadToolApprovalHooks([{ module: './bad.js' }, { module: './good.js' }], {
|
||||
importModule,
|
||||
});
|
||||
expect(n).toBe(1);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(1);
|
||||
});
|
||||
|
||||
test('reload unregisters the previous batch (idempotent, no double-register)', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: goodModule }));
|
||||
await loadToolApprovalHooks([{ module: './a.js' }, { module: './b.js' }], { importModule });
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(2);
|
||||
|
||||
// A reload with a single hook must drop the previous two.
|
||||
await loadToolApprovalHooks([{ module: './a.js' }], { importModule });
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(1);
|
||||
});
|
||||
});
|
||||
112
packages/api/src/agents/hitl/hookLoader.ts
Normal file
112
packages/api/src/agents/hitl/hookLoader.ts
Normal file
|
|
@ -0,0 +1,112 @@
|
|||
import path from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import { logger } from '@librechat/data-schemas';
|
||||
import type { TToolApprovalHookConfig } from 'librechat-data-provider';
|
||||
import type { ToolApprovalHookFactory } from './hooks';
|
||||
import { registerToolApprovalHook } from './hooks';
|
||||
|
||||
/**
|
||||
* The default-export contract a tool-approval hook MODULE must satisfy: a builder that takes
|
||||
* the config entry's static `options` and returns a per-run {@link ToolApprovalHookFactory}.
|
||||
*
|
||||
* // my-hook.js
|
||||
* module.exports = (options) => (context) => async (input) => ({ decision: 'ask' });
|
||||
*/
|
||||
export type ToolApprovalHookModule = (options?: Record<string, unknown>) => ToolApprovalHookFactory;
|
||||
|
||||
export interface LoadToolApprovalHooksOptions {
|
||||
/** Directory to resolve relative module paths against. Defaults to `process.cwd()`. */
|
||||
basePath?: string;
|
||||
/** Override the dynamic importer (used by tests to avoid touching the filesystem). */
|
||||
importModule?: (specifier: string) => Promise<unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Unregister fns for the hooks THIS loader registered. Tracked so a config reload can drop
|
||||
* its previous batch without disturbing hooks registered directly in code.
|
||||
*/
|
||||
let loadedUnregisters: Array<() => void> = [];
|
||||
|
||||
/**
|
||||
* Turn a config `module` string into an importable specifier. A bare package name imports
|
||||
* as-is; a path (relative or absolute) resolves against `basePath` and becomes a `file://`
|
||||
* URL so `import()` accepts it on every platform.
|
||||
*/
|
||||
function resolveModuleSpecifier(spec: string, basePath: string): string {
|
||||
const isPath = spec.startsWith('.') || spec.startsWith('/') || path.isAbsolute(spec);
|
||||
if (!isPath) {
|
||||
return spec;
|
||||
}
|
||||
const absolute = path.isAbsolute(spec) ? spec : path.resolve(basePath, spec);
|
||||
return pathToFileURL(absolute).href;
|
||||
}
|
||||
|
||||
/**
|
||||
* Load + register the programmatic tool-approval hooks declared under
|
||||
* `endpoints.agents.toolApproval.hooks`. Call once at startup (and again on a config reload —
|
||||
* each call first unregisters the previous batch, so it is idempotent across reloads and
|
||||
* never double-registers).
|
||||
*
|
||||
* Robust by design: a bad entry (unimportable module, non-function export, builder that
|
||||
* throws or returns a non-function) is logged and skipped — one misconfigured hook never
|
||||
* crashes startup or blocks the others. Returns the number of hooks successfully registered.
|
||||
*
|
||||
* SECURITY: each `module` is dynamically imported and executed in-process. This is
|
||||
* admin-level config (librechat.yaml); only reference trusted code.
|
||||
*/
|
||||
export async function loadToolApprovalHooks(
|
||||
hooks: TToolApprovalHookConfig[] | undefined,
|
||||
options: LoadToolApprovalHooksOptions = {},
|
||||
): Promise<number> {
|
||||
const basePath = options.basePath ?? process.cwd();
|
||||
const importModule = options.importModule ?? ((specifier: string) => import(specifier));
|
||||
|
||||
// Drop the previous batch (reload safety) WITHOUT clearing code-registered hooks.
|
||||
for (const off of loadedUnregisters) {
|
||||
off();
|
||||
}
|
||||
loadedUnregisters = [];
|
||||
|
||||
if (!Array.isArray(hooks) || hooks.length === 0) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
let registered = 0;
|
||||
for (const entry of hooks) {
|
||||
try {
|
||||
const specifier = resolveModuleSpecifier(entry.module, basePath);
|
||||
const mod = (await importModule(specifier)) as { default?: unknown };
|
||||
const builder = (
|
||||
mod && typeof mod === 'object' && 'default' in mod ? mod.default : mod
|
||||
) as unknown;
|
||||
if (typeof builder !== 'function') {
|
||||
logger.error(
|
||||
`[toolApprovalHooks] Module "${entry.module}" did not export a hook-builder function; skipping`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
const factory = (builder as ToolApprovalHookModule)(entry.options);
|
||||
if (typeof factory !== 'function') {
|
||||
logger.error(
|
||||
`[toolApprovalHooks] Builder from "${entry.module}" did not return a factory function; skipping`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
loadedUnregisters.push(registerToolApprovalHook(factory, { matcher: entry.matcher }));
|
||||
registered++;
|
||||
logger.info(
|
||||
`[toolApprovalHooks] Registered tool-approval hook from "${entry.module}"` +
|
||||
(entry.matcher ? ` (matcher: ${entry.matcher})` : ''),
|
||||
);
|
||||
} catch (err) {
|
||||
logger.error(
|
||||
`[toolApprovalHooks] Failed to load tool-approval hook module "${entry.module}"; skipping`,
|
||||
err,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return registered;
|
||||
}
|
||||
|
|
@ -2,3 +2,4 @@ export * from './policy';
|
|||
export * from './runtime';
|
||||
export * from './resume';
|
||||
export * from './hooks';
|
||||
export * from './hookLoader';
|
||||
|
|
|
|||
|
|
@ -787,6 +787,29 @@ export type ToolApprovalMode = z.infer<typeof toolApprovalModeSchema>;
|
|||
* HITL machinery for this endpoint (no checkpointer, no hooks, no prompts).
|
||||
* This is admin-level; users toggle prompting via `mode: 'bypass'` instead.
|
||||
*/
|
||||
/**
|
||||
* A programmatic tool-approval hook loaded from a module at startup.
|
||||
*
|
||||
* The referenced module's default export must be a builder
|
||||
* `(options?) => ToolApprovalHookFactory` (see `@librechat/api`'s `registerToolApprovalHook`).
|
||||
* Hooks compose with the static `allow`/`deny`/`ask` policy above and can only TIGHTEN it
|
||||
* (the SDK folds decisions `deny → ask → allow`). This is admin-level config — the module is
|
||||
* dynamically imported and executed in-process, so only reference trusted code.
|
||||
*/
|
||||
export const toolApprovalHookConfigSchema = z.object({
|
||||
/**
|
||||
* Module specifier to import: a bare package name (e.g. `@acme/approval-hooks`) or a path —
|
||||
* absolute, or relative to the app root. Its default export is the hook builder.
|
||||
*/
|
||||
module: z.string().min(1),
|
||||
/** Optional regex matched against the tool name; omit to run for every tool. */
|
||||
matcher: z.string().optional(),
|
||||
/** Static options forwarded to the module's builder; the hook's own per-call config. */
|
||||
options: z.record(z.unknown()).optional(),
|
||||
});
|
||||
|
||||
export type TToolApprovalHookConfig = z.infer<typeof toolApprovalHookConfigSchema>;
|
||||
|
||||
export const toolApprovalPolicySchema = z
|
||||
.object({
|
||||
enabled: z.boolean().optional(),
|
||||
|
|
@ -796,6 +819,12 @@ export const toolApprovalPolicySchema = z
|
|||
ask: z.array(z.string()).optional(),
|
||||
/** Optional reason template surfaced in the prompt; `{tool}` is interpolated. */
|
||||
reason: z.string().optional(),
|
||||
/**
|
||||
* Programmatic policy hooks loaded from modules at startup. They layer on top of the
|
||||
* static lists above for dynamic, context-aware decisions the lists can't express
|
||||
* (per-args, per-agent, per-user). See {@link toolApprovalHookConfigSchema}.
|
||||
*/
|
||||
hooks: z.array(toolApprovalHookConfigSchema).optional(),
|
||||
})
|
||||
.optional();
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue