mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🪝 feat: Configurable Tool-Approval Policy via Programmatic Hooks (#14025)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Has been cancelled
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Has been cancelled
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
* 🪝 feat: Programmatic tool-approval hook seam (configurable beyond on/off) Adds a process-wide registry so host code can plug context-aware PreToolUse decision hooks into the tool-approval policy, composing with the static `endpoints.agents.toolApproval` config instead of replacing it. - `registerToolApprovalHook(factory, { matcher? })` — register a factory that builds a PreToolUse hook per run from a ToolApprovalHookContext (userId, conversationId, tenantId, appConfig); return undefined to opt the run out. Returns an unregister fn. - `buildHITLRunWiring(policy, context)` now registers the static-config policy hook as the baseline, then layers each resolved host hook after it. Decisions fold in the SDK as deny > ask > allow, so a host hook can only TIGHTEN a configured ask/deny — it can never silently auto-approve past policy (to loosen, change the static policy). updatedInput / allowedDecisions follow the SDK's last-writer-wins, so host hooks win over the baseline. - `createRun` threads the per-run context (user / conversation / tenant / appConfig) into the wiring; non-HITL and HITL-disabled runs never invoke any factory. This unlocks dynamic policy the static name-lists can't express — per-args (e.g. ask before write_file outside a workspace, the SDK's createWorkspacePolicyHook shape), per-agent, per-user. Inert until tool approval is enabled and the caller is hitlCapable. Tests: registry register/unregister/opt-out/order (hooks.spec.ts) + wiring composition, context passthrough, and disabled-path inertness (runtime.spec.ts). Full HITL suite green. * 🪝 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). * 💄 style: Sort imports in HITL hook spec files (CI sort-imports:check) * 🛡️ fix: Harden tool-approval hook loader (Codex review) Six P2 findings on the hook loader / startup wiring: - CJS/transpiled interop: unwrap a nested `default` (TS/Babel `exports.default = fn` surfaces through import() as `{ default: { default: fn } }`) before rejecting a module, so documented default-export hook modules actually load. - Validate the matcher regex at load time and skip invalid ones — the SDK compiles it with `new RegExp` at run-build time, where a bad pattern would throw out of buildHITLRunWiring and break EVERY HITL run instead of just skipping the one bad hook. - Honor the `enabled` kill switch: startup now passes hooks to the loader only when toolApproval is enabled, so a disabled endpoint imports/runs nothing (and unregisters any prior batch). - Resolve app-root-relative paths without a leading dot: a bare specifier that is a real file under basePath (e.g. `config/hooks/workspace.js`) resolves as a path; scoped/other bare names still import as packages. - Base-config-only: documented that hooks register once process-wide at startup and are NOT reloaded from per-role/user/tenant overrides — encode per-tenant logic inside the hook. - Wire the loader into the clustered startup path (api/server/experimental.js) too, not just the standard server. Tests: CJS-interop unwrap, invalid-matcher skip (+ sibling still loads), and specifier resolution (app-root file / bare package / ./relative). Full HITL suite green. * 🛡️ fix: Read tool-approval hooks from base config in clustered startup (Codex) The clustered experimental.js path read toolApproval from getAppConfig() (which merges DB __base__ overrides with no principal), so a DB override could enable/disable/replace toolApproval.hooks and import those modules in every worker — violating the base-config-only contract and diverging from the standard server. Fetch getAppConfig({ baseOnly: true }) specifically for the hook loader, matching api/server/index.js.
This commit is contained in:
parent
53b3e166d8
commit
424ccffd83
11 changed files with 616 additions and 5 deletions
|
|
@ -20,6 +20,7 @@ const {
|
|||
performStartupChecks,
|
||||
handleJsonParseError,
|
||||
initializeFileStorage,
|
||||
loadToolApprovalHooks,
|
||||
maybeInjectQueryDevtoolsBootstrap,
|
||||
preAuthTenantMiddleware,
|
||||
} = require('@librechat/api');
|
||||
|
|
@ -300,6 +301,16 @@ if (cluster.isMaster) {
|
|||
const appConfig = await getAppConfig();
|
||||
initializeFileStorage(appConfig);
|
||||
initializeGitHubSkillSync(appConfig);
|
||||
// Register configured tool-approval policy hooks (mirrors the standard startup path).
|
||||
// Honors the `enabled` kill switch; hooks are base-config-only, registered process-wide.
|
||||
// Read from the BASE config specifically — `appConfig` above (getAppConfig() with no
|
||||
// principal) still merges DB `__base__` overrides, which must not drive which hook
|
||||
// modules load in every worker (matches api/server/index.js's baseOnly usage).
|
||||
const baseAppConfig = await getAppConfig({ baseOnly: true });
|
||||
const toolApproval = baseAppConfig?.endpoints?.agents?.toolApproval;
|
||||
await loadToolApprovalHooks(toolApproval?.enabled ? toolApproval.hooks : undefined, {
|
||||
basePath: path.resolve(__dirname, '../..'),
|
||||
});
|
||||
expiredFileSweepOptions = { appConfig, loadAppConfig: getAppConfig };
|
||||
startExpiredFileSweepOnce();
|
||||
await performStartupChecks(appConfig);
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ const {
|
|||
createStreamServices,
|
||||
initializeFileStorage,
|
||||
initializeDeploymentSkills,
|
||||
loadToolApprovalHooks,
|
||||
maybeInjectQueryDevtoolsBootstrap,
|
||||
preAuthTenantMiddleware,
|
||||
setupGracefulShutdown,
|
||||
|
|
@ -124,6 +125,16 @@ 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`. Honor the `enabled` kill switch: when tool
|
||||
// approval is off we pass no hooks, so a disabled endpoint imports/runs nothing (and any
|
||||
// previously loaded batch is unregistered). Hooks are read from the BASE config only —
|
||||
// they register once, process-wide; per-user/tenant differences belong inside the hook
|
||||
// (via its context), not in per-override module lists.
|
||||
const toolApproval = appConfig?.endpoints?.agents?.toolApproval;
|
||||
await loadToolApprovalHooks(toolApproval?.enabled ? toolApproval.hooks : undefined, {
|
||||
basePath: path.resolve(__dirname, '../..'),
|
||||
});
|
||||
await runAsSystem(async () => {
|
||||
await performStartupChecks(appConfig);
|
||||
await updateInterfacePermissions({ appConfig, getRoleByName, updateAccessPermissions });
|
||||
|
|
|
|||
153
packages/api/src/agents/hitl/hookLoader.spec.ts
Normal file
153
packages/api/src/agents/hitl/hookLoader.spec.ts
Normal file
|
|
@ -0,0 +1,153 @@
|
|||
import { logger } from '@librechat/data-schemas';
|
||||
import { getRegisteredToolApprovalHookCount, clearToolApprovalHooks } from './hooks';
|
||||
import { loadToolApprovalHooks } from './hookLoader';
|
||||
|
||||
/** 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);
|
||||
});
|
||||
|
||||
test('unwraps a nested default (CJS/transpiled `exports.default = fn` interop)', async () => {
|
||||
// import() of TS/Babel CJS output surfaces as { default: { default: builder } }.
|
||||
const importModule = jest.fn(async () => ({ default: { default: goodModule } }));
|
||||
expect(await loadToolApprovalHooks([{ module: './cjs.js' }], { importModule })).toBe(1);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(1);
|
||||
});
|
||||
|
||||
test('skips an entry with an invalid matcher regex (does not register a throwing pattern)', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: goodModule }));
|
||||
expect(
|
||||
await loadToolApprovalHooks([{ module: './h.js', matcher: '[' }], { importModule }),
|
||||
).toBe(0);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(0);
|
||||
expect(importModule).not.toHaveBeenCalled(); // rejected before import
|
||||
expect(logger.error).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('loads valid hooks even when a sibling entry has a bad matcher', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: goodModule }));
|
||||
const n = await loadToolApprovalHooks(
|
||||
[
|
||||
{ module: './bad.js', matcher: '(' },
|
||||
{ module: './good.js', matcher: 'write_.*' },
|
||||
],
|
||||
{ importModule },
|
||||
);
|
||||
expect(n).toBe(1);
|
||||
});
|
||||
|
||||
describe('module specifier resolution', () => {
|
||||
test('resolves an app-root-relative FILE without a leading dot to a file:// URL', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: goodModule }));
|
||||
// `hookLoader.ts` exists next to this spec — a bare-looking path that is a real file.
|
||||
await loadToolApprovalHooks([{ module: 'hookLoader.ts' }], {
|
||||
importModule,
|
||||
basePath: __dirname,
|
||||
});
|
||||
expect(importModule).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/^file:\/\/.*hookLoader\.ts$/),
|
||||
);
|
||||
});
|
||||
|
||||
test('leaves a bare package specifier untouched when no such file exists', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: goodModule }));
|
||||
await loadToolApprovalHooks([{ module: 'some-approval-hooks-pkg' }], {
|
||||
importModule,
|
||||
basePath: __dirname,
|
||||
});
|
||||
expect(importModule).toHaveBeenCalledWith('some-approval-hooks-pkg');
|
||||
});
|
||||
|
||||
test('resolves a ./ relative path to a file:// URL', async () => {
|
||||
const importModule = jest.fn(async () => ({ default: goodModule }));
|
||||
await loadToolApprovalHooks([{ module: './hooks/x.js' }], {
|
||||
importModule,
|
||||
basePath: '/srv/app',
|
||||
});
|
||||
expect(importModule).toHaveBeenCalledWith('file:///srv/app/hooks/x.js');
|
||||
});
|
||||
});
|
||||
});
|
||||
146
packages/api/src/agents/hitl/hookLoader.ts
Normal file
146
packages/api/src/agents/hitl/hookLoader.ts
Normal file
|
|
@ -0,0 +1,146 @@
|
|||
import path from 'node:path';
|
||||
import { existsSync } from 'node:fs';
|
||||
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. Explicit paths (absolute, or
|
||||
* `./`-relative) resolve against `basePath` and become a `file://` URL so `import()` accepts
|
||||
* them on every platform. A bare specifier prefers a real FILE under `basePath` (so
|
||||
* `config/hooks/workspace.js` works without a leading `./`), falling back to a package
|
||||
* specifier when no such file exists; scoped names (`@scope/pkg`) are always packages.
|
||||
*/
|
||||
function resolveModuleSpecifier(spec: string, basePath: string): string {
|
||||
if (path.isAbsolute(spec)) {
|
||||
return pathToFileURL(spec).href;
|
||||
}
|
||||
if (spec.startsWith('.')) {
|
||||
return pathToFileURL(path.resolve(basePath, spec)).href;
|
||||
}
|
||||
if (!spec.startsWith('@')) {
|
||||
const candidate = path.resolve(basePath, spec);
|
||||
if (existsSync(candidate)) {
|
||||
return pathToFileURL(candidate).href;
|
||||
}
|
||||
}
|
||||
return spec;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 {
|
||||
// Validate the matcher regex up front: the SDK compiles it with `new RegExp` at
|
||||
// run-build time, where a bad pattern would throw out of buildHITLRunWiring and break
|
||||
// EVERY HITL run — here it's skipped like any other bad entry.
|
||||
if (entry.matcher != null) {
|
||||
try {
|
||||
void new RegExp(entry.matcher);
|
||||
} catch (regexErr) {
|
||||
logger.error(
|
||||
`[toolApprovalHooks] Invalid matcher regex ${JSON.stringify(entry.matcher)} for module "${entry.module}"; skipping`,
|
||||
regexErr,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
const specifier = resolveModuleSpecifier(entry.module, basePath);
|
||||
const mod = (await importModule(specifier)) as { default?: unknown };
|
||||
let builder: unknown = mod && typeof mod === 'object' && 'default' in mod ? mod.default : mod;
|
||||
// CJS/transpiled interop: TypeScript/Babel `exports.default = fn` (esModuleInterop)
|
||||
// surfaces through import() as `{ default: { default: fn } }`, so unwrap one more level
|
||||
// before rejecting — otherwise documented "default export" hook modules fail to load.
|
||||
if (
|
||||
builder != null &&
|
||||
typeof builder === 'object' &&
|
||||
'default' in builder &&
|
||||
typeof (builder as { default: unknown }).default === 'function'
|
||||
) {
|
||||
builder = (builder as { default: unknown }).default;
|
||||
}
|
||||
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;
|
||||
}
|
||||
77
packages/api/src/agents/hitl/hooks.spec.ts
Normal file
77
packages/api/src/agents/hitl/hooks.spec.ts
Normal file
|
|
@ -0,0 +1,77 @@
|
|||
import type { ToolApprovalHook } from './hooks';
|
||||
import {
|
||||
registerToolApprovalHook,
|
||||
getRegisteredToolApprovalHookCount,
|
||||
clearToolApprovalHooks,
|
||||
buildToolApprovalHooks,
|
||||
} from './hooks';
|
||||
|
||||
const denyHook: ToolApprovalHook = async () => ({ decision: 'deny' });
|
||||
|
||||
describe('tool-approval hook registry', () => {
|
||||
afterEach(() => clearToolApprovalHooks());
|
||||
|
||||
test('register increments the count and returns an unregister fn', () => {
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(0);
|
||||
const off = registerToolApprovalHook(() => denyHook);
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(1);
|
||||
off();
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(0);
|
||||
});
|
||||
|
||||
test('unregister removes exactly its own registration and is idempotent', () => {
|
||||
const off1 = registerToolApprovalHook(() => denyHook);
|
||||
registerToolApprovalHook(() => denyHook);
|
||||
off1();
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(1);
|
||||
off1(); // already removed — no-op, does not remove the second
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(1);
|
||||
});
|
||||
|
||||
test('clearToolApprovalHooks removes everything', () => {
|
||||
registerToolApprovalHook(() => denyHook);
|
||||
registerToolApprovalHook(() => denyHook);
|
||||
clearToolApprovalHooks();
|
||||
expect(getRegisteredToolApprovalHookCount()).toBe(0);
|
||||
});
|
||||
|
||||
describe('buildToolApprovalHooks', () => {
|
||||
test('resolves factories against context and carries each matcher', () => {
|
||||
registerToolApprovalHook(() => denyHook);
|
||||
registerToolApprovalHook(() => denyHook, { matcher: 'write_.*' });
|
||||
|
||||
const built = buildToolApprovalHooks({ userId: 'bob' });
|
||||
expect(built).toHaveLength(2);
|
||||
expect(built[0].matcher).toBeUndefined();
|
||||
expect(built[1].matcher).toBe('write_.*');
|
||||
expect(typeof built[0].hook).toBe('function');
|
||||
});
|
||||
|
||||
test('drops factories that opt out (return undefined) for the given context', () => {
|
||||
// First hook applies to everyone except admins; second always applies.
|
||||
registerToolApprovalHook((ctx) => (ctx.userId === 'admin' ? undefined : denyHook));
|
||||
registerToolApprovalHook(() => denyHook, { matcher: 'write_.*' });
|
||||
|
||||
expect(buildToolApprovalHooks({ userId: 'bob' })).toHaveLength(2);
|
||||
expect(buildToolApprovalHooks({ userId: 'admin' })).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('invokes factories in registration order', () => {
|
||||
const order: string[] = [];
|
||||
registerToolApprovalHook(() => {
|
||||
order.push('a');
|
||||
return undefined;
|
||||
});
|
||||
registerToolApprovalHook(() => {
|
||||
order.push('b');
|
||||
return undefined;
|
||||
});
|
||||
buildToolApprovalHooks({});
|
||||
expect(order).toEqual(['a', 'b']);
|
||||
});
|
||||
|
||||
test('returns an empty list when nothing is registered', () => {
|
||||
expect(buildToolApprovalHooks({})).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
118
packages/api/src/agents/hitl/hooks.ts
Normal file
118
packages/api/src/agents/hitl/hooks.ts
Normal file
|
|
@ -0,0 +1,118 @@
|
|||
import type { AppConfig } from '@librechat/data-schemas';
|
||||
import type { HookCallback } from '@librechat/agents';
|
||||
|
||||
/**
|
||||
* Per-run context handed to a {@link ToolApprovalHookFactory} when a run is built. It carries
|
||||
* the request-scoped facts the SDK's `PreToolUse` hook input does NOT include — the user, the
|
||||
* conversation, the tenant, and the resolved app config — so a process-wide hook can
|
||||
* specialize its decision per request (e.g. "auto-approve for admins", "stricter for tenant
|
||||
* X"). The SDK input already provides the per-call facts (tool name, args, agent, thread,
|
||||
* turn); this fills the request-identity gap.
|
||||
*/
|
||||
export interface ToolApprovalHookContext {
|
||||
/** The requesting user's id, when authenticated. */
|
||||
userId?: string;
|
||||
/** The conversation (== LangGraph `thread_id`) the run belongs to. */
|
||||
conversationId?: string;
|
||||
/** Tenant id, in multi-tenant deployments. */
|
||||
tenantId?: string;
|
||||
/** The resolved app config for the request. */
|
||||
appConfig?: AppConfig;
|
||||
}
|
||||
|
||||
/**
|
||||
* A programmatic tool-approval hook: a `PreToolUse` callback that decides `allow` / `ask` /
|
||||
* `deny` (and may rewrite the tool args via `updatedInput` or restrict the offered decisions
|
||||
* via `allowedDecisions`) from the FULL live call — tool name, args, executing agent, thread,
|
||||
* turn — not just the static name lists in `endpoints.agents.toolApproval`. Return an empty
|
||||
* object (`{}`) to abstain and fall through to the configured policy / other hooks.
|
||||
*
|
||||
* Hooks COMPOSE with the static policy through the SDK's `PreToolUse` fold, which resolves
|
||||
* decisions `deny` > `ask` > `allow`. A hook can therefore only ever TIGHTEN a configured
|
||||
* `ask` / `deny` — it can never silently auto-approve past policy. To loosen, change the
|
||||
* static policy. (The SDK's own `createWorkspacePolicyHook` is this exact shape.)
|
||||
*/
|
||||
export type ToolApprovalHook = HookCallback<'PreToolUse'>;
|
||||
|
||||
/**
|
||||
* Builds a {@link ToolApprovalHook} for one run from its {@link ToolApprovalHookContext}.
|
||||
* Return `undefined` to opt out of this run entirely (e.g. the policy doesn't apply to this
|
||||
* user, or the app config disables the hook) — cheaper and clearer than a hook that always
|
||||
* abstains. Registered process-wide via {@link registerToolApprovalHook}.
|
||||
*/
|
||||
export type ToolApprovalHookFactory = (
|
||||
context: ToolApprovalHookContext,
|
||||
) => ToolApprovalHook | undefined;
|
||||
|
||||
interface RegisteredHook {
|
||||
factory: ToolApprovalHookFactory;
|
||||
/** Optional regex matched against the tool name (the `PreToolUse` matcher `pattern`). */
|
||||
matcher?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Process-wide registry of tool-approval hook factories. Populated once at startup by host
|
||||
* code / plugins; read per run by {@link buildToolApprovalHooks}. Kept module-private so the
|
||||
* only mutations go through the register/clear API (registration order is preserved, which
|
||||
* the SDK's last-writer-wins precedence for `updatedInput` / `allowedDecisions` relies on).
|
||||
*/
|
||||
const registeredHooks: RegisteredHook[] = [];
|
||||
|
||||
/**
|
||||
* Register a programmatic tool-approval hook (process-wide). Call once at startup. Returns an
|
||||
* unregister function that removes exactly this registration.
|
||||
*
|
||||
* Inert unless tool approval is enabled AND the caller is HITL-capable — hooks only run inside
|
||||
* the `PreToolUse` fold of an HITL run (see {@link buildToolApprovalHooks} /
|
||||
* `buildHITLRunWiring`). They compose with, and can only tighten, the static
|
||||
* `endpoints.agents.toolApproval` policy.
|
||||
*
|
||||
* @param factory Builds the per-run hook from its context; return `undefined` to opt out.
|
||||
* @param options.matcher Optional regex string matched against the tool name — omit to run for
|
||||
* every tool. Patterns are compiled with `new RegExp` by the SDK without a sandbox, so only
|
||||
* register trusted / length-bounded patterns.
|
||||
*/
|
||||
export function registerToolApprovalHook(
|
||||
factory: ToolApprovalHookFactory,
|
||||
options: { matcher?: string } = {},
|
||||
): () => void {
|
||||
const entry: RegisteredHook = { factory, matcher: options.matcher };
|
||||
registeredHooks.push(entry);
|
||||
return () => {
|
||||
const index = registeredHooks.indexOf(entry);
|
||||
if (index >= 0) {
|
||||
registeredHooks.splice(index, 1);
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
/** Number of currently-registered hook factories (diagnostics / tests). */
|
||||
export function getRegisteredToolApprovalHookCount(): number {
|
||||
return registeredHooks.length;
|
||||
}
|
||||
|
||||
/** Remove every registered hook. Test/teardown helper. */
|
||||
export function clearToolApprovalHooks(): void {
|
||||
registeredHooks.length = 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the registered hook factories against a run's {@link ToolApprovalHookContext} into
|
||||
* concrete `PreToolUse` hooks (each with its optional tool-name matcher). Factories that
|
||||
* return `undefined` (opt out for this run) are dropped. Registration order is preserved.
|
||||
*
|
||||
* Consumed by `buildHITLRunWiring`, which registers these AFTER the static-policy hook so a
|
||||
* host hook's `updatedInput` / `allowedDecisions` win the SDK's last-writer-wins precedence.
|
||||
*/
|
||||
export function buildToolApprovalHooks(
|
||||
context: ToolApprovalHookContext,
|
||||
): Array<{ hook: ToolApprovalHook; matcher?: string }> {
|
||||
const built: Array<{ hook: ToolApprovalHook; matcher?: string }> = [];
|
||||
for (const { factory, matcher } of registeredHooks) {
|
||||
const hook = factory(context);
|
||||
if (hook) {
|
||||
built.push({ hook, matcher });
|
||||
}
|
||||
}
|
||||
return built;
|
||||
}
|
||||
|
|
@ -1,3 +1,5 @@
|
|||
export * from './policy';
|
||||
export * from './runtime';
|
||||
export * from './resume';
|
||||
export * from './hooks';
|
||||
export * from './hookLoader';
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { HookRegistry } from '@librechat/agents';
|
||||
import { registerToolApprovalHook, clearToolApprovalHooks } from './hooks';
|
||||
import { buildHITLRunWiring } from './runtime';
|
||||
|
||||
describe('buildHITLRunWiring', () => {
|
||||
|
|
@ -27,3 +28,37 @@ describe('buildHITLRunWiring', () => {
|
|||
expect(wiring?.hooks.getMatchers('PreToolUse')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildHITLRunWiring host-hook composition', () => {
|
||||
afterEach(() => clearToolApprovalHooks());
|
||||
|
||||
test('registers the static policy hook PLUS each registered host hook', () => {
|
||||
registerToolApprovalHook(() => async () => ({ decision: 'deny' }));
|
||||
registerToolApprovalHook(() => async () => ({ decision: 'ask' }), { matcher: 'write_.*' });
|
||||
const wiring = buildHITLRunWiring({ enabled: true });
|
||||
// 1 static baseline + 2 host hooks
|
||||
expect(wiring?.hooks.getMatchers('PreToolUse')).toHaveLength(3);
|
||||
});
|
||||
|
||||
test('a factory that opts out (returns undefined) is not registered', () => {
|
||||
registerToolApprovalHook(() => undefined);
|
||||
const wiring = buildHITLRunWiring({ enabled: true });
|
||||
expect(wiring?.hooks.getMatchers('PreToolUse')).toHaveLength(1); // only the static baseline
|
||||
});
|
||||
|
||||
test('does not invoke host-hook factories when HITL is disabled', () => {
|
||||
const factory = jest.fn(() => undefined);
|
||||
registerToolApprovalHook(factory);
|
||||
expect(buildHITLRunWiring({ enabled: false })).toBeUndefined();
|
||||
expect(factory).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('passes the run context to each factory', () => {
|
||||
const factory = jest.fn(() => undefined);
|
||||
registerToolApprovalHook(factory);
|
||||
buildHITLRunWiring({ enabled: true }, { userId: 'u1', conversationId: 'c1' });
|
||||
expect(factory).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ userId: 'u1', conversationId: 'c1' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
import { HookRegistry, createToolPolicyHook } from '@librechat/agents';
|
||||
import type { TToolApprovalPolicy } from 'librechat-data-provider';
|
||||
import type { ToolApprovalHookContext } from './hooks';
|
||||
import { isHITLEnabled, mapToolApprovalPolicy } from './policy';
|
||||
import { buildToolApprovalHooks } from './hooks';
|
||||
|
||||
/**
|
||||
* The HITL fragment spread onto a `RunConfig` when tool approval is enabled.
|
||||
|
|
@ -20,22 +22,37 @@ export interface HITLRunWiring {
|
|||
* when HITL is disabled (the default) — in which case the run attaches nothing
|
||||
* and behaves exactly as it did before this feature.
|
||||
*
|
||||
* The returned `hooks` registry carries a single `PreToolUse` policy hook built
|
||||
* from {@link mapToolApprovalPolicy}. An enabled policy with no allow/deny/ask
|
||||
* lists falls through to `mode: 'default'`, i.e. every tool prompts — the safe
|
||||
* default for "HITL on, nothing else specified".
|
||||
* The returned `hooks` registry carries the static-config `PreToolUse` policy hook built
|
||||
* from {@link mapToolApprovalPolicy} (an enabled policy with no allow/deny/ask lists falls
|
||||
* through to `mode: 'default'`, i.e. every tool prompts — the safe default for "HITL on,
|
||||
* nothing else specified"), PLUS any host-registered programmatic hooks
|
||||
* ({@link registerToolApprovalHook}) resolved against `context`. The static hook is
|
||||
* registered first as the baseline; host hooks layer after it. Decisions fold in the SDK
|
||||
* as `deny` > `ask` > `allow`, so a host hook can only TIGHTEN the configured policy.
|
||||
*/
|
||||
export function buildHITLRunWiring(
|
||||
policy: TToolApprovalPolicy | undefined,
|
||||
context: ToolApprovalHookContext = {},
|
||||
): HITLRunWiring | undefined {
|
||||
if (!isHITLEnabled(policy)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const registry = new HookRegistry();
|
||||
// Static config-driven policy (mode/allow/deny/ask) — the baseline.
|
||||
registry.register('PreToolUse', {
|
||||
hooks: [createToolPolicyHook(mapToolApprovalPolicy(policy) ?? {})],
|
||||
});
|
||||
|
||||
// Host-registered programmatic hooks — context-aware, layered after the baseline so their
|
||||
// `updatedInput` / `allowedDecisions` win the SDK's last-writer-wins precedence. Each can
|
||||
// carry its own tool-name matcher; the SDK still folds decisions deny > ask > allow.
|
||||
for (const { hook, matcher } of buildToolApprovalHooks(context)) {
|
||||
registry.register(
|
||||
'PreToolUse',
|
||||
matcher ? { pattern: matcher, hooks: [hook] } : { hooks: [hook] },
|
||||
);
|
||||
}
|
||||
|
||||
return { humanInTheLoop: { enabled: true }, hooks: registry };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1193,7 +1193,14 @@ export async function createRun({
|
|||
// would pause with no approval surface or resume endpoint, and the route would emit a
|
||||
// normal final response / `[DONE]` with the tool call dangling. Only AgentClient (chat +
|
||||
// resume) passes `hitlCapable`; without it the run is identical to the no-HITL path.
|
||||
const hitl = hitlCapable ? buildHITLRunWiring(toolApprovalPolicy) : undefined;
|
||||
const hitl = hitlCapable
|
||||
? buildHITLRunWiring(toolApprovalPolicy, {
|
||||
userId: user?.id,
|
||||
conversationId: requestBody?.conversationId,
|
||||
tenantId: tenantId ?? user?.tenantId,
|
||||
appConfig,
|
||||
})
|
||||
: undefined;
|
||||
if (hitl) {
|
||||
const checkpointer = await getAgentCheckpointer(agentsEndpointConfig?.checkpointer);
|
||||
graphConfig.compileOptions = { ...graphConfig.compileOptions, checkpointer };
|
||||
|
|
|
|||
|
|
@ -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,17 @@ 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}.
|
||||
*
|
||||
* BASE-CONFIG ONLY: hooks are imported + registered once, process-wide, at server
|
||||
* startup — they are NOT reloaded from per-role/user/tenant admin overrides. Encode
|
||||
* per-user/tenant behavior INSIDE the hook (via its runtime context), not by varying the
|
||||
* module list per override. Honored only when `enabled` is true.
|
||||
*/
|
||||
hooks: z.array(toolApprovalHookConfigSchema).optional(),
|
||||
})
|
||||
.optional();
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue