mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(hooks): retire legacy inline handler config (#120851)
This commit is contained in:
committed by
GitHub
parent
73bdb4b924
commit
4c3bf14466
@@ -6,7 +6,7 @@ read_when:
|
||||
title: "Hooks"
|
||||
---
|
||||
|
||||
Hooks are small scripts that run inside the Gateway when agent events fire: commands like `/new`, `/reset`, `/stop`, session compaction, gateway lifecycle, and message flow. They are discovered from directories and managed with `openclaw hooks`. The Gateway loads internal hooks only after you enable hooks or configure at least one hook entry, hook pack, legacy handler, or extra hook directory.
|
||||
Hooks are small scripts that run inside the Gateway when agent events fire: commands like `/new`, `/reset`, `/stop`, session compaction, gateway lifecycle, and message flow. They are discovered from directories and managed with `openclaw hooks`. The Gateway loads internal hooks only after you enable hooks or configure at least one hook entry, hook pack, or extra hook directory.
|
||||
|
||||
There are two kinds of hooks in OpenClaw:
|
||||
|
||||
@@ -211,7 +211,7 @@ Hooks are discovered from four sources:
|
||||
|
||||
Workspace hooks can add new hook names but cannot override bundled, managed, or plugin-provided hooks with the same name.
|
||||
|
||||
The Gateway skips internal hook discovery on startup until internal hooks are configured. Enable a bundled or managed hook with `openclaw hooks enable <name>`, install a hook pack, or set `hooks.internal.enabled=true` to opt in. When you enable one named hook, the Gateway loads only that hook's handler; `hooks.internal.enabled=true`, extra hook directories, and legacy handlers opt into broad discovery.
|
||||
The Gateway skips internal hook discovery on startup until internal hooks are configured. Enable a bundled or managed hook with `openclaw hooks enable <name>`, install a hook pack, or set `hooks.internal.enabled=true` to opt in. Named entries remain an allowlist even when the master flag is true. A bare `hooks.internal.enabled=true` with no named entries enables broad discovery; non-empty extra hook directories and hook-pack installs that do not declare their hook names are also open-ended.
|
||||
|
||||
### Hook packs
|
||||
|
||||
@@ -367,9 +367,9 @@ Extra hook directories:
|
||||
}
|
||||
```
|
||||
|
||||
<Note>
|
||||
The legacy `hooks.internal.handlers` array config format is still supported for backwards compatibility, but new hooks should use the discovery-based system.
|
||||
</Note>
|
||||
<Warning>
|
||||
`hooks.internal.handlers` is retired and is no longer loaded or accepted by normal config validation. Before running `openclaw doctor --fix`, move each registered module into a managed or workspace hook directory with `HOOK.md` and a handler file. Doctor removes the retired registrations; it does not create executable hook files. For a legacy-only configuration with `hooks.internal.enabled: true`, Doctor also removes `enabled` to avoid enabling unrelated discovered hooks. Canonical entries, non-empty extra directories, and explicit `enabled: false` are preserved.
|
||||
</Warning>
|
||||
|
||||
## CLI reference
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
// Hook command tests cover metadata config keys and missing-hook exit status.
|
||||
import { Command } from "commander";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { resolveConfiguredInternalHookNames } from "../hooks/configured.js";
|
||||
import type { HookStatusEntry, HookStatusReport } from "../hooks/hooks-status.js";
|
||||
import { createEmptyInstallChecks } from "./requirements-test-fixtures.js";
|
||||
import { createCliRuntimeCapture } from "./test-runtime-capture.js";
|
||||
@@ -15,6 +17,11 @@ const mocks = vi.hoisted(() => ({
|
||||
}));
|
||||
|
||||
const capture = createCliRuntimeCapture();
|
||||
const readConfigMachineStateMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../state/config-machine-state.js", () => ({
|
||||
readConfigMachineState: readConfigMachineStateMock,
|
||||
}));
|
||||
|
||||
vi.mock("../agents/agent-scope.js", () => ({
|
||||
resolveAgentWorkspaceDir: () => "/tmp/openclaw-hook-workspace",
|
||||
@@ -128,6 +135,7 @@ describe("hooks CLI metadata config keys", () => {
|
||||
mocks.getRuntimeConfig.mockReturnValue(sourceConfig);
|
||||
mocks.readConfigFileSnapshot.mockResolvedValue({ sourceConfig, hash: "config-hash" });
|
||||
mocks.replaceConfigFile.mockResolvedValue(undefined);
|
||||
readConfigMachineStateMock.mockReturnValue(undefined);
|
||||
});
|
||||
|
||||
it.each([
|
||||
@@ -156,6 +164,10 @@ describe("hooks CLI metadata config keys", () => {
|
||||
},
|
||||
baseHash: "config-hash",
|
||||
});
|
||||
const writtenConfig = mocks.replaceConfigFile.mock.calls[0]?.[0]?.nextConfig as OpenClawConfig;
|
||||
expect(resolveConfiguredInternalHookNames(writtenConfig)).toEqual(
|
||||
new Set(testCase.enabled ? ["metadata-key"] : []),
|
||||
);
|
||||
expect(capture.runtimeLogs.at(-1)).toContain("display-name");
|
||||
expect(mocks.requestExitAfterOneShotOutput).toHaveBeenCalledWith(capture.defaultRuntime, 0);
|
||||
expect(mocks.callGateway).not.toHaveBeenCalled();
|
||||
|
||||
@@ -167,6 +167,26 @@ const legacyConfigMigrationForTest = vi.hoisted(() => {
|
||||
changes.push("Moved heartbeat to agents.defaults.heartbeat and channels.defaults.heartbeat.");
|
||||
}
|
||||
|
||||
const internalHooks = asRecord(asRecord(next.hooks)?.internal);
|
||||
if (internalHooks && "handlers" in internalHooks) {
|
||||
delete internalHooks.handlers;
|
||||
changes.push(
|
||||
"Removed retired hooks.internal.handlers registrations; hook files must be migrated separately.",
|
||||
);
|
||||
const entries = asRecord(internalHooks.entries);
|
||||
const extraDirs = asRecord(internalHooks.load)?.extraDirs;
|
||||
const hasNamedEntries = Boolean(entries && Object.keys(entries).length > 0);
|
||||
const hasExtraDirs =
|
||||
Array.isArray(extraDirs) &&
|
||||
extraDirs.some((dir) => typeof dir === "string" && dir.trim().length > 0);
|
||||
if (internalHooks.enabled === true && !hasNamedEntries && !hasExtraDirs) {
|
||||
delete internalHooks.enabled;
|
||||
changes.push(
|
||||
"Removed legacy-only hooks.internal.enabled to avoid enabling broad hook discovery.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const gateway = asRecord(next.gateway);
|
||||
if (gateway?.bind === "0.0.0.0") {
|
||||
gateway.bind = "lan";
|
||||
@@ -425,6 +445,14 @@ vi.mock("../config/legacy.js", () => {
|
||||
'agents.defaults.sandbox.perSession is legacy; use agents.defaults.sandbox.scope. Run "openclaw doctor --fix".',
|
||||
);
|
||||
}
|
||||
const internalHooks = asRecord(asRecord(root.hooks)?.internal);
|
||||
if (internalHooks && "handlers" in internalHooks) {
|
||||
addIssue(
|
||||
issues,
|
||||
["hooks", "internal", "handlers"],
|
||||
'hooks.internal.handlers is retired. Move each module to a managed/workspace hook directory with HOOK.md + handler file before running "openclaw doctor --fix"; the fix removes retired registrations and does not materialize executable files.',
|
||||
);
|
||||
}
|
||||
|
||||
const channels = asRecord(root.channels);
|
||||
for (const [channelId, channelRaw] of Object.entries(channels ?? {})) {
|
||||
@@ -3459,8 +3487,9 @@ describe("doctor config flow", () => {
|
||||
expect(legacyMessages).toContain("tools.web.x_search.apiKey:");
|
||||
expect(legacyMessages).toContain("plugins.entries.xai.config.webSearch.apiKey");
|
||||
expect(legacyMessages).toContain("hooks.internal.handlers:");
|
||||
expect(legacyMessages).toContain("HOOK.md + handler.js");
|
||||
expect(legacyMessages).toContain("does not rewrite this shape automatically");
|
||||
expect(legacyMessages).toContain("HOOK.md + handler file");
|
||||
expect(legacyMessages).toContain("before running");
|
||||
expect(legacyMessages).toContain("does not materialize executable files");
|
||||
expect(legacyMessages).toContain("session.threadBindings.ttlHours");
|
||||
expect(legacyMessages).toContain("session.threadBindings.idleHours");
|
||||
expect(legacyMessages).toContain("session.maintenance.rotateBytes");
|
||||
@@ -3485,6 +3514,45 @@ describe("doctor config flow", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
[false, "Doctor changes preview", false],
|
||||
[true, "Doctor changes", true],
|
||||
] as const)(
|
||||
"previews and repairs retired internal hook registrations (repair=%s)",
|
||||
async (repair, panelTitle, shouldWriteConfig) => {
|
||||
const noteSpy = resetTerminalNoteMock();
|
||||
try {
|
||||
const result = await runDoctorConfigWithInput({
|
||||
config: {
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
handlers: [{ event: "command:new", module: "hooks/legacy-handler.js" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
repair,
|
||||
run: loadAndMaybeMigrateDoctorConfig,
|
||||
});
|
||||
|
||||
expect(
|
||||
(result.cfg.hooks?.internal as Record<string, unknown> | undefined)?.handlers,
|
||||
).toBeUndefined();
|
||||
expect(result.cfg.hooks?.internal?.enabled).toBeUndefined();
|
||||
expect(result.shouldWriteConfig).toBe(shouldWriteConfig);
|
||||
expect(
|
||||
noteSpy.mock.calls.some(
|
||||
([message, title]) =>
|
||||
title === panelTitle &&
|
||||
message.includes("Removed retired hooks.internal.handlers registrations"),
|
||||
),
|
||||
).toBe(true);
|
||||
} finally {
|
||||
noteSpy.mockClear();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("titles the legacy migration panel as a preview when --fix is not passed (#80817)", async () => {
|
||||
const noteSpy = resetTerminalNoteMock();
|
||||
try {
|
||||
|
||||
@@ -34,12 +34,6 @@ import { isSingleTopLevelIncludeMigration } from "./doctor/shared/include-migrat
|
||||
import { normalizeCompatibilityConfigValues } from "./doctor/shared/legacy-config-core-migrate.js";
|
||||
import type { DoctorPluginMetadataSnapshotState } from "./doctor/shared/plugin-metadata-snapshot-scope.js";
|
||||
|
||||
function hasLegacyInternalHookHandlers(raw: unknown): boolean {
|
||||
const handlers = (raw as { hooks?: { internal?: { handlers?: unknown } } })?.hooks?.internal
|
||||
?.handlers;
|
||||
return Array.isArray(handlers) && handlers.length > 0;
|
||||
}
|
||||
|
||||
function collectInvalidHookTransformsDirWarnings(
|
||||
cfg: OpenClawConfig,
|
||||
configPath: string,
|
||||
@@ -314,16 +308,6 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
|
||||
note(legacyIssueLines.join("\n"), "Legacy config keys detected");
|
||||
}
|
||||
emitDoctorChangesPanel(legacyStep.changeLines, shouldRepair);
|
||||
if (hasLegacyInternalHookHandlers(snapshot.parsed)) {
|
||||
note(
|
||||
[
|
||||
"- hooks.internal.handlers: legacy inline hook modules are no longer part of the public config surface.",
|
||||
"- Migrate each entry to a managed or workspace hook directory with HOOK.md + handler.js, then enable it through hooks.internal.entries.<hookKey> as needed.",
|
||||
"- openclaw doctor --fix does not rewrite this shape automatically.",
|
||||
].join("\n"),
|
||||
"Legacy config keys detected",
|
||||
);
|
||||
}
|
||||
const hookTransformsDirWarnings = collectInvalidHookTransformsDirWarnings(
|
||||
state.cfg,
|
||||
snapshot.path,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { findLegacyConfigIssues } from "../../../config/legacy.js";
|
||||
import { LEGACY_CONFIG_MIGRATIONS_RUNTIME_CRON } from "./legacy-config-migrations.runtime.cron.js";
|
||||
import { LEGACY_CONFIG_MIGRATIONS_RUNTIME_GATEWAY } from "./legacy-config-migrations.runtime.gateway.js";
|
||||
import { LEGACY_CONFIG_MIGRATIONS_RUNTIME_MCP } from "./legacy-config-migrations.runtime.mcp.js";
|
||||
@@ -44,6 +45,113 @@ function getPath(value: unknown, path: string): unknown {
|
||||
}
|
||||
|
||||
describe("retired runtime config migrations", () => {
|
||||
it.each([
|
||||
["a normal registration", [{ event: "command:new", module: "hooks/legacy.js" }]],
|
||||
["an empty array", []],
|
||||
["null", null],
|
||||
["a malformed scalar", "hooks/legacy.js"],
|
||||
["a malformed object", { module: "hooks/legacy.js" }],
|
||||
])("detects hooks.internal.handlers by key presence for %s", (_label, handlers) => {
|
||||
const issues = findLegacyConfigIssues({ hooks: { internal: { handlers } } });
|
||||
|
||||
expect(issues).toContainEqual({
|
||||
path: "hooks.internal.handlers",
|
||||
message: expect.stringContaining("hooks.internal.handlers is retired"),
|
||||
});
|
||||
});
|
||||
|
||||
it("removes retired hook registrations while preserving canonical siblings", () => {
|
||||
const migration = LEGACY_CONFIG_MIGRATIONS_RUNTIME_RETIRED.find(
|
||||
(candidate) => candidate.id === "runtime.retired-internal-hook-handlers",
|
||||
);
|
||||
expect(migration).toBeDefined();
|
||||
const raw = {
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
handlers: [{ event: "command:new", module: "hooks/legacy.js" }],
|
||||
entries: { canonical: { enabled: true } },
|
||||
load: { extraDirs: ["/opt/openclaw/hooks"] },
|
||||
sibling: "preserved",
|
||||
},
|
||||
},
|
||||
};
|
||||
const changes: string[] = [];
|
||||
|
||||
migration?.apply(raw, changes);
|
||||
|
||||
expect(raw.hooks.internal).toEqual({
|
||||
enabled: true,
|
||||
entries: { canonical: { enabled: true } },
|
||||
load: { extraDirs: ["/opt/openclaw/hooks"] },
|
||||
sibling: "preserved",
|
||||
});
|
||||
expect(changes).toEqual([
|
||||
"Removed retired hooks.internal.handlers registrations; hook files must be migrated separately.",
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["no canonical siblings", {}, {}],
|
||||
[
|
||||
"empty canonical siblings",
|
||||
{ entries: {}, load: { extraDirs: [] } },
|
||||
{
|
||||
entries: {},
|
||||
load: { extraDirs: [] },
|
||||
},
|
||||
],
|
||||
["blank extra directories", { load: { extraDirs: [" "] } }, { load: { extraDirs: [" "] } }],
|
||||
])("removes legacy-only enabled for %s", (_label, siblings, expected) => {
|
||||
const migration = LEGACY_CONFIG_MIGRATIONS_RUNTIME_RETIRED.find(
|
||||
(candidate) => candidate.id === "runtime.retired-internal-hook-handlers",
|
||||
);
|
||||
const raw = {
|
||||
hooks: { internal: { enabled: true, handlers: [], ...structuredClone(siblings) } },
|
||||
};
|
||||
const changes: string[] = [];
|
||||
|
||||
migration?.apply(raw, changes);
|
||||
|
||||
expect(raw.hooks.internal).toEqual(expected);
|
||||
expect(changes).toEqual([
|
||||
"Removed retired hooks.internal.handlers registrations; hook files must be migrated separately.",
|
||||
"Removed legacy-only hooks.internal.enabled to avoid enabling broad hook discovery.",
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["named entries", { enabled: true, entries: { canonical: { enabled: false } } }],
|
||||
["extra directories", { enabled: true, load: { extraDirs: ["/opt/openclaw/hooks"] } }],
|
||||
["explicit disablement", { enabled: false }],
|
||||
])("preserves canonical enabled state for %s", (_label, expected) => {
|
||||
const migration = LEGACY_CONFIG_MIGRATIONS_RUNTIME_RETIRED.find(
|
||||
(candidate) => candidate.id === "runtime.retired-internal-hook-handlers",
|
||||
);
|
||||
const raw = { hooks: { internal: { ...structuredClone(expected), handlers: null } } };
|
||||
const changes: string[] = [];
|
||||
|
||||
migration?.apply(raw, changes);
|
||||
|
||||
expect(raw.hooks.internal).toEqual(expected);
|
||||
const rerunChanges: string[] = [];
|
||||
migration?.apply(raw, rerunChanges);
|
||||
expect(rerunChanges).toEqual([]);
|
||||
});
|
||||
|
||||
it("explains the required manual migration before doctor removes registrations", () => {
|
||||
const migration = LEGACY_CONFIG_MIGRATIONS_RUNTIME_RETIRED.find(
|
||||
(candidate) => candidate.id === "runtime.retired-internal-hook-handlers",
|
||||
);
|
||||
const message = migration?.legacyRules?.[0]?.message ?? "";
|
||||
|
||||
expect(message).toContain("managed/workspace hook directory");
|
||||
expect(message).toContain("HOOK.md + handler file");
|
||||
expect(message.indexOf("Move each module")).toBeLessThan(message.indexOf("doctor --fix"));
|
||||
expect(message).toContain("removes retired registrations");
|
||||
expect(message).toContain("does not materialize executable files");
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
"strips the retired compaction gate while keeping an enabled byte threshold",
|
||||
|
||||
@@ -419,6 +419,41 @@ function migrateFinalLayoutKills(raw: Record<string, unknown>, changes: string[]
|
||||
}
|
||||
|
||||
export const LEGACY_CONFIG_MIGRATIONS_RUNTIME_RETIRED: LegacyConfigMigrationSpec[] = [
|
||||
defineLegacyConfigMigration({
|
||||
id: "runtime.retired-internal-hook-handlers",
|
||||
describe: "Remove retired internal hook handler registrations",
|
||||
legacyRules: [
|
||||
{
|
||||
path: ["hooks", "internal", "handlers"],
|
||||
message:
|
||||
'hooks.internal.handlers is retired. Move each module to a managed/workspace hook directory with HOOK.md + handler file before running "openclaw doctor --fix"; the fix removes retired registrations and does not materialize executable files.',
|
||||
},
|
||||
],
|
||||
apply: (raw, changes) => {
|
||||
const internal = getRecord(getRecord(raw.hooks)?.internal);
|
||||
if (!internal || !Object.hasOwn(internal, "handlers")) {
|
||||
return;
|
||||
}
|
||||
|
||||
delete internal.handlers;
|
||||
changes.push(
|
||||
"Removed retired hooks.internal.handlers registrations; hook files must be migrated separately.",
|
||||
);
|
||||
|
||||
const entries = getRecord(internal.entries);
|
||||
const extraDirs = getRecord(internal.load)?.extraDirs;
|
||||
const hasNamedEntries = Boolean(entries && Object.keys(entries).length > 0);
|
||||
const hasExtraDirs =
|
||||
Array.isArray(extraDirs) &&
|
||||
extraDirs.some((dir) => typeof dir === "string" && dir.trim().length > 0);
|
||||
if (internal.enabled === true && !hasNamedEntries && !hasExtraDirs) {
|
||||
delete internal.enabled;
|
||||
changes.push(
|
||||
"Removed legacy-only hooks.internal.enabled to avoid enabling broad hook discovery.",
|
||||
);
|
||||
}
|
||||
},
|
||||
}),
|
||||
defineLegacyConfigMigration({
|
||||
id: "runtime.doctor-tier-eval-tranche",
|
||||
describe: "Consolidate approved tier-eval configuration surfaces",
|
||||
|
||||
@@ -48,33 +48,22 @@ describe("config hooks module paths", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects absolute hooks.internal.handlers[].module", () => {
|
||||
it.each([
|
||||
["a former handler registration", [{ event: "command:new", module: "hooks/handler.mjs" }]],
|
||||
["an empty array", []],
|
||||
["a malformed value", "hooks/handler.mjs"],
|
||||
])("rejects retired hooks.internal.handlers for %s", (_label, handlers) => {
|
||||
expectRejectedIssuePath(
|
||||
{
|
||||
agents: { entries: { openclaw: {} } },
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
handlers: [{ event: "command:new", module: "/tmp/handler.mjs" }],
|
||||
handlers,
|
||||
},
|
||||
},
|
||||
},
|
||||
"hooks.internal.handlers.0.module",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects escaping hooks.internal.handlers[].module", () => {
|
||||
expectRejectedIssuePath(
|
||||
{
|
||||
agents: { entries: { openclaw: {} } },
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
handlers: [{ event: "command:new", module: "../handler.mjs" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
"hooks.internal.handlers.0.module",
|
||||
"hooks.internal",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -21,8 +21,6 @@ import { OpenClawSchema } from "./zod-schema.js";
|
||||
|
||||
type ConfigSchema = Record<string, unknown>;
|
||||
|
||||
const LEGACY_HIDDEN_PUBLIC_PATHS = ["hooks.internal.handlers"] as const;
|
||||
|
||||
/**
|
||||
* Recursively walk a JSON Schema object and apply field docs using dot-path
|
||||
* matching. Existing titles/descriptions (for example from Zod metadata) are
|
||||
@@ -126,50 +124,6 @@ function stripChannelSchema(schema: ConfigSchema): ConfigSchema {
|
||||
return next;
|
||||
}
|
||||
|
||||
function stripObjectPropertyPath(schema: ConfigSchema, path: readonly string[]): void {
|
||||
const root = asSchemaObject(schema);
|
||||
if (!root || path.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
let current: JsonSchemaObject | null = root;
|
||||
for (const segment of path.slice(0, -1)) {
|
||||
current = asSchemaObject(current?.properties?.[segment]);
|
||||
if (!current) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const key = path[path.length - 1];
|
||||
if (!current?.properties || !key) {
|
||||
return;
|
||||
}
|
||||
delete current.properties[key];
|
||||
if (Array.isArray(current.required)) {
|
||||
current.required = current.required.filter((entry) => entry !== key);
|
||||
}
|
||||
}
|
||||
|
||||
function stripLegacyCompatSchemaPaths(schema: ConfigSchema): ConfigSchema {
|
||||
const next = cloneSchema(schema);
|
||||
for (const path of LEGACY_HIDDEN_PUBLIC_PATHS) {
|
||||
stripObjectPropertyPath(next, path.split("."));
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
function stripLegacyCompatHints(hints: ConfigUiHints): ConfigUiHints {
|
||||
const next: ConfigUiHints = { ...hints };
|
||||
for (const path of LEGACY_HIDDEN_PUBLIC_PATHS) {
|
||||
for (const key of Object.keys(next)) {
|
||||
if (key === path || key.startsWith(`${path}.`) || key.startsWith(`${path}[`)) {
|
||||
delete next[key];
|
||||
}
|
||||
}
|
||||
}
|
||||
return next;
|
||||
}
|
||||
|
||||
let baseConfigSchemaStablePayload: BaseConfigSchemaStablePayload | null = null;
|
||||
|
||||
function computeBaseConfigSchemaStablePayload(): BaseConfigSchemaStablePayload {
|
||||
@@ -196,15 +150,13 @@ function computeBaseConfigSchemaStablePayload(): BaseConfigSchemaStablePayload {
|
||||
"",
|
||||
isSensitiveUrlConfigPath,
|
||||
);
|
||||
const publicSchema = stripLegacyCompatSchemaPaths(stripChannelSchema(schema));
|
||||
const publicSchema = stripChannelSchema(schema);
|
||||
const stablePayload = {
|
||||
schema: publicSchema,
|
||||
uiHints: applyDerivedTags(
|
||||
applyResolvedConfigTierHints(
|
||||
publicSchema,
|
||||
stripLegacyCompatHints(
|
||||
applyDerivedTags(applySensitiveUrlHints(baseHints, sensitiveUrlPaths)),
|
||||
),
|
||||
applyDerivedTags(applySensitiveUrlHints(baseHints, sensitiveUrlPaths)),
|
||||
),
|
||||
),
|
||||
version: VERSION,
|
||||
|
||||
@@ -69,14 +69,6 @@ export const HookMappingSchema = z
|
||||
.strict()
|
||||
.optional();
|
||||
|
||||
const InternalHookHandlerSchema = z
|
||||
.object({
|
||||
event: z.string(),
|
||||
module: SafeRelativeModulePathSchema,
|
||||
export: z.string().optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
const HookConfigSchema = z
|
||||
.object({
|
||||
enabled: z.boolean().optional(),
|
||||
@@ -90,7 +82,6 @@ const HookConfigSchema = z
|
||||
export const InternalHooksSchema = z
|
||||
.object({
|
||||
enabled: z.boolean().optional(),
|
||||
handlers: z.array(InternalHookHandlerSchema).optional(),
|
||||
entries: z.record(z.string(), HookConfigSchema).optional(),
|
||||
load: z
|
||||
.object({
|
||||
|
||||
@@ -4,19 +4,15 @@
|
||||
* This handler demonstrates how to create a hook that logs all command events
|
||||
* to a centralized log file for audit/debugging purposes.
|
||||
*
|
||||
* To enable this handler, add it to your config:
|
||||
* Enable this bundled hook with `openclaw hooks enable command-logger` or config:
|
||||
*
|
||||
* ```json
|
||||
* {
|
||||
* "hooks": {
|
||||
* "internal": {
|
||||
* "enabled": true,
|
||||
* "handlers": [
|
||||
* {
|
||||
* "event": "command",
|
||||
* "module": "./hooks/handlers/command-logger.ts"
|
||||
* }
|
||||
* ]
|
||||
* "entries": {
|
||||
* "command-logger": { "enabled": true }
|
||||
* }
|
||||
* }
|
||||
* }
|
||||
* }
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
// Configured hook tests cover the closed allowlist and open discovery decisions.
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { resolveConfiguredInternalHookNames } from "./configured.js";
|
||||
|
||||
const readConfigMachineStateMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../state/config-machine-state.js", () => ({
|
||||
readConfigMachineState: readConfigMachineStateMock,
|
||||
}));
|
||||
|
||||
describe("resolveConfiguredInternalHookNames", () => {
|
||||
beforeEach(() => {
|
||||
readConfigMachineStateMock.mockReset();
|
||||
readConfigMachineStateMock.mockReturnValue(undefined);
|
||||
});
|
||||
|
||||
it("keeps CLI-shaped named entries closed when the master flag is enabled", () => {
|
||||
expect(
|
||||
resolveConfiguredInternalHookNames({
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: {
|
||||
enabled: { enabled: true },
|
||||
disabled: { enabled: false },
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toEqual(new Set(["enabled"]));
|
||||
|
||||
expect(
|
||||
resolveConfiguredInternalHookNames({
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: { disabled: { enabled: false } },
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toEqual(new Set());
|
||||
});
|
||||
|
||||
it("keeps a bare master enable open for broad discovery", () => {
|
||||
expect(
|
||||
resolveConfiguredInternalHookNames({
|
||||
hooks: { internal: { enabled: true } },
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps extra directories open-ended even with named entries", () => {
|
||||
expect(
|
||||
resolveConfiguredInternalHookNames({
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: { named: { enabled: true } },
|
||||
load: { extraDirs: ["/opt/openclaw/hooks"] },
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("uses declared install hook names as an allowlist", () => {
|
||||
readConfigMachineStateMock.mockReturnValue({
|
||||
pack: { source: "path", hooks: ["installed-one", "installed-two"] },
|
||||
});
|
||||
|
||||
expect(resolveConfiguredInternalHookNames({})).toEqual(
|
||||
new Set(["installed-one", "installed-two"]),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps installs with unknown dynamic hook names open-ended", () => {
|
||||
readConfigMachineStateMock.mockReturnValue({ pack: { source: "path" } });
|
||||
|
||||
expect(resolveConfiguredInternalHookNames({})).toBeNull();
|
||||
});
|
||||
|
||||
it("lets explicit disablement override every discovery surface", () => {
|
||||
readConfigMachineStateMock.mockReturnValue({ pack: { source: "path" } });
|
||||
const config = {
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: false,
|
||||
entries: { named: { enabled: true } },
|
||||
load: { extraDirs: ["/opt/openclaw/hooks"] },
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig;
|
||||
|
||||
expect(resolveConfiguredInternalHookNames(config)).toEqual(new Set());
|
||||
});
|
||||
});
|
||||
+14
-16
@@ -2,7 +2,6 @@
|
||||
import type { HookConfig, HookInstallRecord } from "../config/types.hooks.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { readConfigMachineState } from "../state/config-machine-state.js";
|
||||
import { getLegacyInternalHookHandlers } from "./legacy-config.js";
|
||||
|
||||
function hasEnabledFlag(entry: HookConfig | undefined): boolean {
|
||||
return entry?.enabled !== false;
|
||||
@@ -23,7 +22,7 @@ function readConfiguredInstalls(): Record<string, HookInstallRecord> | undefined
|
||||
return readConfigMachineState<Record<string, HookInstallRecord>>("hooks.internal.installs");
|
||||
}
|
||||
|
||||
/** Return whether config can load any internal hooks, including legacy handlers. */
|
||||
/** Return whether config can load any internal hooks. */
|
||||
export function hasConfiguredInternalHooks(config: OpenClawConfig): boolean {
|
||||
const internal = config.hooks?.internal;
|
||||
const installs = readConfiguredInstalls();
|
||||
@@ -45,27 +44,26 @@ export function hasConfiguredInternalHooks(config: OpenClawConfig): boolean {
|
||||
if (hasConfiguredInstalls(installs)) {
|
||||
return true;
|
||||
}
|
||||
return getLegacyInternalHookHandlers(config).length > 0;
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Resolve explicitly configured internal hook names; null means all/discovered hooks may load. */
|
||||
export function resolveConfiguredInternalHookNames(config: OpenClawConfig): Set<string> | null {
|
||||
const internal = config.hooks?.internal;
|
||||
const installs = readConfiguredInstalls();
|
||||
if (!internal) {
|
||||
return hasConfiguredInstalls(installs) ? null : new Set();
|
||||
}
|
||||
if (internal.enabled === false) {
|
||||
if (internal?.enabled === false) {
|
||||
return new Set();
|
||||
}
|
||||
if (internal.enabled === true) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const names = new Set<string>();
|
||||
for (const [name, entry] of Object.entries(internal.entries ?? {})) {
|
||||
let hasNamedEntries = false;
|
||||
for (const [name, entry] of Object.entries(internal?.entries ?? {})) {
|
||||
const trimmed = name.trim();
|
||||
if (trimmed && hasEnabledFlag(entry)) {
|
||||
if (!trimmed) {
|
||||
continue;
|
||||
}
|
||||
hasNamedEntries = true;
|
||||
if (hasEnabledFlag(entry)) {
|
||||
names.add(trimmed);
|
||||
}
|
||||
}
|
||||
@@ -84,11 +82,11 @@ export function resolveConfiguredInternalHookNames(config: OpenClawConfig): Set<
|
||||
}
|
||||
}
|
||||
|
||||
if ((internal.load?.extraDirs ?? []).some((dir) => dir.trim().length > 0)) {
|
||||
if ((internal?.load?.extraDirs ?? []).some((dir) => dir.trim().length > 0)) {
|
||||
return null;
|
||||
}
|
||||
if (getLegacyInternalHookHandlers(config).length > 0) {
|
||||
return null;
|
||||
if (hasNamedEntries || names.size > 0) {
|
||||
return names;
|
||||
}
|
||||
return names;
|
||||
return internal?.enabled === true ? null : names;
|
||||
}
|
||||
|
||||
@@ -1,20 +0,0 @@
|
||||
// Legacy hook config helpers convert older hook records into current config shape.
|
||||
type LegacyInternalHookHandler = {
|
||||
event: string;
|
||||
module: string;
|
||||
export?: string;
|
||||
};
|
||||
|
||||
type LegacyInternalHooksCarrier = {
|
||||
hooks?: {
|
||||
internal?: {
|
||||
handlers?: LegacyInternalHookHandler[];
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
/** Read legacy hooks.internal.handlers entries for backward-compatible config detection. */
|
||||
export function getLegacyInternalHookHandlers(config: unknown): LegacyInternalHookHandler[] {
|
||||
const handlers = (config as LegacyInternalHooksCarrier)?.hooks?.internal?.handlers;
|
||||
return Array.isArray(handlers) ? handlers : [];
|
||||
}
|
||||
+95
-227
@@ -3,7 +3,6 @@ import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { stripAnsi } from "../../packages/terminal-core/src/ansi.js";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { setLoggerOverride } from "../logging/logger.js";
|
||||
import { loggingState } from "../logging/state.js";
|
||||
@@ -53,18 +52,25 @@ describe("loader", () => {
|
||||
hookName: string;
|
||||
handlerCode?: string;
|
||||
events?: string[];
|
||||
exportName?: string;
|
||||
hookKey?: string;
|
||||
}): Promise<string> {
|
||||
const sourceDir = params.sourceDir ?? path.join(tmpDir, "hooks");
|
||||
const hookDir = path.join(sourceDir, params.hookName);
|
||||
await fs.mkdir(hookDir, { recursive: true });
|
||||
const events = params.events ?? ["command:new"];
|
||||
const metadata = {
|
||||
events,
|
||||
...(params.exportName ? { export: params.exportName } : {}),
|
||||
...(params.hookKey ? { hookKey: params.hookKey } : {}),
|
||||
};
|
||||
await fs.writeFile(
|
||||
path.join(hookDir, "HOOK.md"),
|
||||
[
|
||||
"---",
|
||||
`name: ${params.hookName}`,
|
||||
`description: ${params.hookName} test hook`,
|
||||
`metadata: {"openclaw":{"events":${JSON.stringify(events)}}}`,
|
||||
`metadata: ${JSON.stringify({ openclaw: metadata })}`,
|
||||
"---",
|
||||
"",
|
||||
`# ${params.hookName}`,
|
||||
@@ -80,45 +86,8 @@ describe("loader", () => {
|
||||
return hookDir;
|
||||
}
|
||||
|
||||
async function writeHandlerModule(
|
||||
fileName: string,
|
||||
code = "export default async function() {}",
|
||||
): Promise<string> {
|
||||
const handlerPath = path.join(tmpDir, fileName);
|
||||
await fs.writeFile(handlerPath, code, "utf-8");
|
||||
return handlerPath;
|
||||
}
|
||||
|
||||
function withLegacyInternalHookHandlers(
|
||||
config: OpenClawConfig,
|
||||
handlers?: Array<{ event: string; module: string; export?: string }>,
|
||||
): OpenClawConfig {
|
||||
if (!handlers) {
|
||||
return config;
|
||||
}
|
||||
return {
|
||||
...config,
|
||||
hooks: {
|
||||
...config.hooks,
|
||||
internal: {
|
||||
...config.hooks?.internal,
|
||||
handlers,
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
}
|
||||
|
||||
function createEnabledHooksConfig(
|
||||
handlers?: Array<{ event: string; module: string; export?: string }>,
|
||||
): OpenClawConfig {
|
||||
return withLegacyInternalHookHandlers(
|
||||
{
|
||||
hooks: {
|
||||
internal: { enabled: true },
|
||||
},
|
||||
},
|
||||
handlers,
|
||||
);
|
||||
function createEnabledHooksConfig(): OpenClawConfig {
|
||||
return { hooks: { internal: { enabled: true } } };
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
@@ -166,14 +135,6 @@ describe("loader", () => {
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
const createLegacyHandlerConfig = () =>
|
||||
createEnabledHooksConfig([
|
||||
{
|
||||
event: "command:new",
|
||||
module: "legacy-handler.js",
|
||||
},
|
||||
]);
|
||||
|
||||
const expectNoCommandHookRegistration = async (cfg: OpenClawConfig) => {
|
||||
const count = await loadInternalHooks(cfg, tmpDir);
|
||||
expect(count).toBe(0);
|
||||
@@ -181,28 +142,11 @@ describe("loader", () => {
|
||||
};
|
||||
|
||||
it("should return 0 when hooks are explicitly disabled", async () => {
|
||||
for (const cfg of [
|
||||
{
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig,
|
||||
withLegacyInternalHookHandlers(
|
||||
{
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
} satisfies OpenClawConfig,
|
||||
[],
|
||||
),
|
||||
]) {
|
||||
const count = await loadInternalHooks(cfg, tmpDir);
|
||||
expect(count).toBe(0);
|
||||
}
|
||||
const count = await loadInternalHooks(
|
||||
{ hooks: { internal: { enabled: false } } } satisfies OpenClawConfig,
|
||||
tmpDir,
|
||||
);
|
||||
expect(count).toBe(0);
|
||||
});
|
||||
|
||||
it("skips hook discovery until internal hooks are configured", async () => {
|
||||
@@ -225,6 +169,7 @@ describe("loader", () => {
|
||||
{
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: {
|
||||
"keep-hook": { enabled: true },
|
||||
},
|
||||
@@ -241,6 +186,34 @@ describe("loader", () => {
|
||||
expect(event.messages).toEqual(["keep-hook"]);
|
||||
});
|
||||
|
||||
it("matches configured names against metadata hook keys", async () => {
|
||||
const hooksDir = path.join(tmpDir, "managed-hooks");
|
||||
await writeDiscoveredHook({
|
||||
sourceDir: hooksDir,
|
||||
hookName: "display-name",
|
||||
hookKey: "metadata-key",
|
||||
});
|
||||
await writeDiscoveredHook({ sourceDir: hooksDir, hookName: "skip-hook" });
|
||||
|
||||
const count = await loadInternalHooks(
|
||||
{
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: { "metadata-key": { enabled: true } },
|
||||
},
|
||||
},
|
||||
},
|
||||
tmpDir,
|
||||
{ managedHooksDir: hooksDir, bundledHooksDir: "/nonexistent/bundled/hooks" },
|
||||
);
|
||||
|
||||
expect(count).toBe(1);
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
await triggerInternalHook(event);
|
||||
expect(event.messages).toEqual(["display-name"]);
|
||||
});
|
||||
|
||||
it("registers unknown event keys anyway (advisory warning, not a load failure)", async () => {
|
||||
const hooksDir = path.join(tmpDir, "managed-hooks");
|
||||
await writeDiscoveredHook({
|
||||
@@ -274,58 +247,6 @@ describe("loader", () => {
|
||||
expect(event.messages).toEqual(["typo-hook"]);
|
||||
});
|
||||
|
||||
it("registers legacy handler events with unknown keys anyway (advisory)", async () => {
|
||||
const handlerPath = await writeHandlerModule("legacy-typo-handler.js");
|
||||
|
||||
const cfg = createEnabledHooksConfig([
|
||||
{ event: "command:nwe", module: path.basename(handlerPath) },
|
||||
]);
|
||||
|
||||
const count = await loadInternalHooks(cfg, tmpDir);
|
||||
expect(count).toBe(1);
|
||||
expect(getRegisteredEventKeys()).toContain("command:nwe");
|
||||
});
|
||||
|
||||
it("should load multiple handlers", async () => {
|
||||
// Create test handler modules
|
||||
const handler1Path = await writeHandlerModule("handler1.js");
|
||||
const handler2Path = await writeHandlerModule("handler2.js");
|
||||
|
||||
const cfg = createEnabledHooksConfig([
|
||||
{ event: "command:new", module: path.basename(handler1Path) },
|
||||
{ event: "command:stop", module: path.basename(handler2Path) },
|
||||
]);
|
||||
|
||||
const count = await loadInternalHooks(cfg, tmpDir);
|
||||
expect(count).toBe(2);
|
||||
|
||||
const keys = getRegisteredEventKeys();
|
||||
expect(keys).toContain("command:new");
|
||||
expect(keys).toContain("command:stop");
|
||||
});
|
||||
|
||||
it("loads legacy handler modules from dot-prefixed workspace paths", async () => {
|
||||
await fs.mkdir(path.join(tmpDir, "..hooks"), { recursive: true });
|
||||
await writeHandlerModule(
|
||||
path.join("..hooks", "legacy-handler.js"),
|
||||
'export default async function(event) { event.messages.push("dot-prefixed-hook"); }\n',
|
||||
);
|
||||
|
||||
const cfg = createEnabledHooksConfig([
|
||||
{
|
||||
event: "command:new",
|
||||
module: path.join("..hooks", "legacy-handler.js"),
|
||||
},
|
||||
]);
|
||||
|
||||
const count = await loadInternalHooks(cfg, tmpDir);
|
||||
expect(count).toBe(1);
|
||||
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
await triggerInternalHook(event);
|
||||
expect(event.messages).toEqual(["dot-prefixed-hook"]);
|
||||
});
|
||||
|
||||
it("preserves plugin-registered hooks when workspace hooks reload", async () => {
|
||||
const pluginHandler = vi.fn();
|
||||
registerInternalHook("gateway:startup", pluginHandler);
|
||||
@@ -340,20 +261,23 @@ describe("loader", () => {
|
||||
});
|
||||
|
||||
it("replaces prior workspace hook registrations instead of duplicating them", async () => {
|
||||
await writeHandlerModule(
|
||||
"legacy-handler.js",
|
||||
'export default async function(event) { event.messages.push("reloadable-hook"); }\n',
|
||||
);
|
||||
|
||||
const cfg = createEnabledHooksConfig([
|
||||
{
|
||||
event: "command:new",
|
||||
module: "legacy-handler.js",
|
||||
const hooksDir = path.join(tmpDir, "managed-hooks");
|
||||
await writeDiscoveredHook({
|
||||
sourceDir: hooksDir,
|
||||
hookName: "reloadable-hook",
|
||||
});
|
||||
const cfg = {
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: { "reloadable-hook": { enabled: true } },
|
||||
},
|
||||
},
|
||||
]);
|
||||
} satisfies OpenClawConfig;
|
||||
const options = { managedHooksDir: hooksDir, bundledHooksDir: "/nonexistent/bundled/hooks" };
|
||||
|
||||
expect(await loadInternalHooks(cfg, tmpDir)).toBe(1);
|
||||
expect(await loadInternalHooks(cfg, tmpDir)).toBe(1);
|
||||
expect(await loadInternalHooks(cfg, tmpDir, options)).toBe(1);
|
||||
expect(await loadInternalHooks(cfg, tmpDir, options)).toBe(1);
|
||||
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
await triggerInternalHook(event);
|
||||
@@ -366,49 +290,50 @@ describe("loader", () => {
|
||||
});
|
||||
|
||||
it("should support named exports", async () => {
|
||||
// Create a handler module with named export
|
||||
const handlerCode = `
|
||||
export const myHandler = async function(event) {
|
||||
// Named export handler
|
||||
}
|
||||
`;
|
||||
const handlerPath = await writeHandlerModule("named-export.js", handlerCode);
|
||||
|
||||
const cfg = createEnabledHooksConfig([
|
||||
{
|
||||
event: "command:new",
|
||||
module: path.basename(handlerPath),
|
||||
export: "myHandler",
|
||||
const hooksDir = path.join(tmpDir, "managed-hooks");
|
||||
await writeDiscoveredHook({
|
||||
sourceDir: hooksDir,
|
||||
hookName: "named-export",
|
||||
exportName: "myHandler",
|
||||
handlerCode: "export const myHandler = async function() {};\n",
|
||||
});
|
||||
const cfg = {
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: { "named-export": { enabled: true } },
|
||||
},
|
||||
},
|
||||
]);
|
||||
} satisfies OpenClawConfig;
|
||||
|
||||
const count = await loadInternalHooks(cfg, tmpDir);
|
||||
const count = await loadInternalHooks(cfg, tmpDir, {
|
||||
managedHooksDir: hooksDir,
|
||||
bundledHooksDir: "/nonexistent/bundled/hooks",
|
||||
});
|
||||
expect(count).toBe(1);
|
||||
});
|
||||
|
||||
it("should treat invalid handlers as non-loadable", async () => {
|
||||
const badExportPath = await writeHandlerModule(
|
||||
"bad-export.js",
|
||||
'export default "not a function";',
|
||||
);
|
||||
const hooksDir = path.join(tmpDir, "managed-hooks");
|
||||
await writeDiscoveredHook({
|
||||
sourceDir: hooksDir,
|
||||
hookName: "bad-export",
|
||||
handlerCode: 'export default "not a function";\n',
|
||||
});
|
||||
|
||||
for (const cfg of [
|
||||
createEnabledHooksConfig([
|
||||
{
|
||||
event: "command:new",
|
||||
module: "missing-handler.js",
|
||||
const count = await loadInternalHooks(
|
||||
{
|
||||
hooks: {
|
||||
internal: {
|
||||
enabled: true,
|
||||
entries: { "bad-export": { enabled: true } },
|
||||
},
|
||||
},
|
||||
]),
|
||||
createEnabledHooksConfig([
|
||||
{
|
||||
event: "command:new",
|
||||
module: path.basename(badExportPath),
|
||||
},
|
||||
]),
|
||||
]) {
|
||||
const count = await loadInternalHooks(cfg, tmpDir);
|
||||
expect(count).toBe(0);
|
||||
}
|
||||
},
|
||||
tmpDir,
|
||||
{ managedHooksDir: hooksDir, bundledHooksDir: "/nonexistent/bundled/hooks" },
|
||||
);
|
||||
expect(count).toBe(0);
|
||||
});
|
||||
|
||||
it("keeps workspace hooks disabled by default until explicitly enabled", async () => {
|
||||
@@ -468,20 +393,6 @@ describe("loader", () => {
|
||||
await expectNoCommandHookRegistration(createEnabledHooksConfig());
|
||||
});
|
||||
|
||||
it("rejects legacy handler modules that escape workspace via symlink", async () => {
|
||||
const outsideHandlerPath = path.join(fixtureRoot, `outside-legacy-${caseId}.js`);
|
||||
await fs.writeFile(outsideHandlerPath, "export default async function() {}", "utf-8");
|
||||
|
||||
const linkedHandlerPath = path.join(tmpDir, "legacy-handler.js");
|
||||
try {
|
||||
await fs.symlink(outsideHandlerPath, linkedHandlerPath);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
|
||||
await expectNoCommandHookRegistration(createLegacyHandlerConfig());
|
||||
});
|
||||
|
||||
it("rejects directory hook handlers that escape hook dir via hardlink", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
@@ -516,49 +427,6 @@ describe("loader", () => {
|
||||
await expectNoCommandHookRegistration(createEnabledHooksConfig());
|
||||
});
|
||||
|
||||
it("rejects legacy handler modules that escape workspace via hardlink", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const outsideHandlerPath = path.join(fixtureRoot, `outside-legacy-hardlink-${caseId}.js`);
|
||||
await fs.writeFile(outsideHandlerPath, "export default async function() {}", "utf-8");
|
||||
|
||||
const linkedHandlerPath = path.join(tmpDir, "legacy-handler.js");
|
||||
try {
|
||||
await fs.link(outsideHandlerPath, linkedHandlerPath);
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
await expectNoCommandHookRegistration(createLegacyHandlerConfig());
|
||||
});
|
||||
|
||||
it("sanitizes control characters in loader error logs", async () => {
|
||||
const error = loggingState.rawConsole?.error;
|
||||
expect(error).toBeTypeOf("function");
|
||||
|
||||
const cfg = createEnabledHooksConfig([
|
||||
{
|
||||
event: "command:new",
|
||||
module: `${tmpDir}\u001b[31m\nforged-log`,
|
||||
},
|
||||
]);
|
||||
|
||||
await expectNoCommandHookRegistration(cfg);
|
||||
|
||||
const messages = stripAnsi(
|
||||
(error as ReturnType<typeof vi.fn>).mock.calls
|
||||
.map((call) => String(call[0] ?? ""))
|
||||
.join("\n"),
|
||||
);
|
||||
expect(messages).toContain("forged-log");
|
||||
expect(messages).not.toContain("\u001b[31m");
|
||||
expect(messages).not.toContain("\nforged-log");
|
||||
});
|
||||
|
||||
it("keeps managed hooks active when a workspace hook reuses the same name", async () => {
|
||||
const managedHooksDir = path.join(tmpDir, "managed-hooks");
|
||||
await writeDiscoveredHook({
|
||||
|
||||
+7
-111
@@ -6,7 +6,6 @@
|
||||
*/
|
||||
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { openRootFile } from "../infra/boundary-file-read.js";
|
||||
@@ -16,11 +15,11 @@ import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { resolveGlobalSingleton } from "../shared/global-singleton.js";
|
||||
import { shouldIncludeHook } from "./config.js";
|
||||
import { hasConfiguredInternalHooks, resolveConfiguredInternalHookNames } from "./configured.js";
|
||||
import { resolveHookKey } from "./frontmatter.js";
|
||||
import { buildImportUrl } from "./import-url.js";
|
||||
import { isKnownInternalHookEventKey } from "./internal-hook-types.js";
|
||||
import type { InternalHookHandler } from "./internal-hooks.js";
|
||||
import { registerInternalHook, unregisterInternalHook } from "./internal-hooks.js";
|
||||
import { getLegacyInternalHookHandlers } from "./legacy-config.js";
|
||||
import { resolveFunctionModuleExport } from "./module-loader.js";
|
||||
import { loadWorkspaceHookEntries } from "./workspace.js";
|
||||
|
||||
@@ -41,15 +40,6 @@ function safeLogValue(value: string): string {
|
||||
return sanitizeForLog(value);
|
||||
}
|
||||
|
||||
function isNonEmptyRelativePathInsideRoot(relativePath: string): boolean {
|
||||
return (
|
||||
relativePath !== "" &&
|
||||
relativePath !== ".." &&
|
||||
!relativePath.startsWith(`..${path.sep}`) &&
|
||||
!path.isAbsolute(relativePath)
|
||||
);
|
||||
}
|
||||
|
||||
function maybeWarnTrustedHookSource(source: string): void {
|
||||
if (source === "openclaw-workspace") {
|
||||
log.warn(
|
||||
@@ -77,9 +67,7 @@ function resetLoadedInternalHooks(): void {
|
||||
/**
|
||||
* Load and register all hook handlers
|
||||
*
|
||||
* Loads hooks from both:
|
||||
* 1. Directory-based discovery (bundled, managed, workspace)
|
||||
* 2. Legacy config handlers (backwards compatibility)
|
||||
* Loads hooks from directory-based discovery (bundled, managed, workspace).
|
||||
*
|
||||
* @param cfg - OpenClaw configuration
|
||||
* @param workspaceDir - Workspace directory for hook discovery
|
||||
@@ -110,7 +98,6 @@ export async function loadInternalHooks(
|
||||
let loadedCount = 0;
|
||||
const configuredNames = resolveConfiguredInternalHookNames(cfg);
|
||||
|
||||
// 1. Load hooks from directories (new system)
|
||||
try {
|
||||
const hookEntries = loadWorkspaceHookEntries(workspaceDir, {
|
||||
config: cfg,
|
||||
@@ -120,8 +107,11 @@ export async function loadInternalHooks(
|
||||
|
||||
// Filter by eligibility
|
||||
const eligible = hookEntries.filter((entry) => {
|
||||
if (configuredNames && !configuredNames.has(entry.hook.name)) {
|
||||
return false;
|
||||
if (configuredNames) {
|
||||
const hookKey = resolveHookKey(entry.hook.name, entry);
|
||||
if (!configuredNames.has(entry.hook.name) && !configuredNames.has(hookKey)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return shouldIncludeHook({ entry, config: cfg });
|
||||
});
|
||||
@@ -208,99 +198,5 @@ export async function loadInternalHooks(
|
||||
log.error(`Failed to load directory-based hooks: ${safeLogValue(formatErrorMessage(err))}`);
|
||||
}
|
||||
|
||||
// 2. Load legacy config handlers (backwards compatibility)
|
||||
const handlers = getLegacyInternalHookHandlers(cfg);
|
||||
for (const handlerConfig of handlers) {
|
||||
try {
|
||||
// Legacy handler paths: keep them workspace-relative.
|
||||
const rawModule = handlerConfig.module.trim();
|
||||
if (!rawModule) {
|
||||
log.error("Handler module path is empty");
|
||||
continue;
|
||||
}
|
||||
if (path.isAbsolute(rawModule)) {
|
||||
log.error(
|
||||
`Handler module path must be workspace-relative (got absolute path): ${safeLogValue(rawModule)}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const baseDir = path.resolve(workspaceDir);
|
||||
const modulePath = path.resolve(baseDir, rawModule);
|
||||
const baseDirReal = safeRealpathSync(baseDir);
|
||||
if (!baseDirReal) {
|
||||
log.error(
|
||||
`Workspace directory is no longer readable while loading hooks: ${safeLogValue(baseDir)}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const modulePathSafe = safeRealpathSync(modulePath);
|
||||
if (!modulePathSafe) {
|
||||
log.error(
|
||||
`Handler module path could not be resolved with realpath: ${safeLogValue(rawModule)}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const rel = path.relative(baseDirReal, modulePathSafe);
|
||||
if (!isNonEmptyRelativePathInsideRoot(rel)) {
|
||||
log.error(`Handler module path must stay within workspaceDir: ${safeLogValue(rawModule)}`);
|
||||
continue;
|
||||
}
|
||||
const opened = await openRootFile({
|
||||
absolutePath: modulePathSafe,
|
||||
rootPath: baseDirReal,
|
||||
boundaryLabel: "workspace directory",
|
||||
});
|
||||
if (!opened.ok) {
|
||||
log.error(
|
||||
`Handler module path fails boundary checks under workspaceDir: ${safeLogValue(rawModule)}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
const safeModulePath = opened.path;
|
||||
fs.closeSync(opened.fd);
|
||||
log.warn(
|
||||
`Loading legacy internal hook module from workspace path ${safeLogValue(rawModule)}. Legacy hook modules are trusted local code.`,
|
||||
);
|
||||
|
||||
// Legacy handlers are always workspace-relative, so use mtime-based cache busting
|
||||
const importUrl = buildImportUrl(safeModulePath, "openclaw-workspace");
|
||||
const mod = (await import(importUrl)) as Record<string, unknown>;
|
||||
|
||||
// Get the handler function
|
||||
const exportName = handlerConfig.export ?? "default";
|
||||
const handler = resolveFunctionModuleExport<InternalHookHandler>({
|
||||
mod,
|
||||
exportName,
|
||||
});
|
||||
|
||||
if (!handler) {
|
||||
log.error(
|
||||
`Handler '${safeLogValue(exportName)}' from ${safeLogValue(modulePath)} is not a function`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Same advisory typo check as directory-discovered hooks above.
|
||||
if (!isKnownInternalHookEventKey(handlerConfig.event)) {
|
||||
log.warn(
|
||||
`Legacy hook handler ${safeLogValue(rawModule)} subscribes to event ` +
|
||||
`${safeLogValue(handlerConfig.event)} not emitted by OpenClaw core — ` +
|
||||
`likely a typo; unless a plugin emits it, the hook never fires. ` +
|
||||
`Known events: https://docs.openclaw.ai/automation/hooks`,
|
||||
);
|
||||
}
|
||||
registerInternalHook(handlerConfig.event, handler);
|
||||
loadedHookRegistrations.push({ event: handlerConfig.event, handler });
|
||||
log.debug(
|
||||
`Registered hook (legacy): ${safeLogValue(handlerConfig.event)} -> ${safeLogValue(modulePath)}${exportName !== "default" ? `#${safeLogValue(exportName)}` : ""}`,
|
||||
);
|
||||
loadedCount++;
|
||||
} catch (err) {
|
||||
log.error(
|
||||
`Failed to load hook handler from ${safeLogValue(handlerConfig.module)}: ${safeLogValue(formatErrorMessage(err))}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return loadedCount;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user