fix(core): unify session-key normalization and plugin boundary checks

This commit is contained in:
Peter Steinberger
2026-02-26 12:40:57 +00:00
parent e3385a6578
commit 4b71de384c
13 changed files with 182 additions and 34 deletions

View File

@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { resolveCronAgentSessionKey } from "./run.js";
import { resolveCronAgentSessionKey } from "./session-key.js";
describe("resolveCronAgentSessionKey", () => {
it("builds an agent-scoped key for legacy aliases", () => {

View File

@@ -40,11 +40,7 @@ import {
import type { AgentDefaultsConfig } from "../../config/types.js";
import { registerAgentRunContext } from "../../infra/agent-events.js";
import { logWarn } from "../../logger.js";
import {
buildAgentMainSessionKey,
normalizeAgentId,
parseAgentSessionKey,
} from "../../routing/session-key.js";
import { normalizeAgentId } from "../../routing/session-key.js";
import {
buildSafeExternalPrompt,
detectSuspiciousPatterns,
@@ -67,6 +63,7 @@ import {
pickSummaryFromPayloads,
resolveHeartbeatAckMaxChars,
} from "./helpers.js";
import { resolveCronAgentSessionKey } from "./session-key.js";
import { resolveCronSession } from "./session.js";
import { resolveCronSkillsSnapshot } from "./skills-snapshot.js";
@@ -647,18 +644,3 @@ export async function runCronIsolatedAgentTurn(params: {
return resolveRunOutcome({ delivered, deliveryAttempted });
}
export function resolveCronAgentSessionKey(params: {
sessionKey: string;
agentId: string;
}): string {
const baseSessionKey = params.sessionKey.trim();
const normalizedBaseSessionKey = baseSessionKey.toLowerCase();
if (parseAgentSessionKey(normalizedBaseSessionKey)) {
return normalizedBaseSessionKey;
}
return buildAgentMainSessionKey({
agentId: params.agentId,
mainKey: baseSessionKey,
});
}

View File

@@ -0,0 +1,13 @@
import { toAgentStoreSessionKey } from "../../routing/session-key.js";
export function resolveCronAgentSessionKey(params: {
sessionKey: string;
agentId: string;
mainKey?: string | undefined;
}): string {
return toAgentStoreSessionKey({
agentId: params.agentId,
requestKey: params.sessionKey.trim(),
mainKey: params.mainKey,
});
}

View File

@@ -7,6 +7,7 @@ import { createIMessageTestPlugin } from "../test-utils/imessage-test-plugin.js"
import {
extractHookToken,
isHookAgentAllowed,
normalizeHookDispatchSessionKey,
resolveHookSessionKey,
resolveHookTargetAgentId,
normalizeAgentPayload,
@@ -280,6 +281,24 @@ describe("gateway hooks helpers", () => {
expect(resolvedKey).toEqual({ ok: true, value: "hook:ingress" });
});
test("normalizeHookDispatchSessionKey strips duplicate target agent prefix", () => {
expect(
normalizeHookDispatchSessionKey({
sessionKey: "agent:hooks:slack:channel:c123",
targetAgentId: "hooks",
}),
).toBe("slack:channel:c123");
});
test("normalizeHookDispatchSessionKey preserves non-target agent scoped keys", () => {
expect(
normalizeHookDispatchSessionKey({
sessionKey: "agent:main:slack:channel:c123",
targetAgentId: "hooks",
}),
).toBe("agent:main:slack:channel:c123");
});
test("resolveHooksConfig validates defaultSessionKey and generated fallback against prefixes", () => {
expect(() =>
resolveHooksConfig({

View File

@@ -5,7 +5,7 @@ import { listChannelPlugins } from "../channels/plugins/index.js";
import type { ChannelId } from "../channels/plugins/types.js";
import type { OpenClawConfig } from "../config/config.js";
import { readJsonBodyWithLimit, requestBodyErrorToText } from "../infra/http-body.js";
import { normalizeAgentId } from "../routing/session-key.js";
import { normalizeAgentId, parseAgentSessionKey } from "../routing/session-key.js";
import { normalizeMessageChannel } from "../utils/message-channel.js";
import { type HookMappingResolved, resolveHookMappings } from "./hooks-mapping.js";
@@ -332,6 +332,25 @@ export function resolveHookSessionKey(params: {
return { ok: true, value: generated };
}
export function normalizeHookDispatchSessionKey(params: {
sessionKey: string;
targetAgentId: string | undefined;
}): string {
const trimmed = params.sessionKey.trim();
if (!trimmed || !params.targetAgentId) {
return trimmed;
}
const parsed = parseAgentSessionKey(trimmed);
if (!parsed) {
return trimmed;
}
const targetAgentId = normalizeAgentId(params.targetAgentId);
if (parsed.agentId !== targetAgentId) {
return `agent:${parsed.agentId}:${parsed.rest}`;
}
return parsed.rest;
}
export function normalizeAgentPayload(payload: Record<string, unknown>):
| {
ok: true;

View File

@@ -49,6 +49,7 @@ import {
normalizeHookHeaders,
normalizeWakePayload,
readJsonBody,
normalizeHookDispatchSessionKey,
resolveHookSessionKey,
resolveHookTargetAgentId,
resolveHookChannel,
@@ -355,10 +356,14 @@ export function createHooksRequestHandler(
sendJson(res, 400, { ok: false, error: sessionKey.error });
return true;
}
const targetAgentId = resolveHookTargetAgentId(hooksConfig, normalized.value.agentId);
const runId = dispatchAgentHook({
...normalized.value,
sessionKey: sessionKey.value,
agentId: resolveHookTargetAgentId(hooksConfig, normalized.value.agentId),
sessionKey: normalizeHookDispatchSessionKey({
sessionKey: sessionKey.value,
targetAgentId,
}),
agentId: targetAgentId,
});
sendJson(res, 202, { ok: true, runId });
return true;
@@ -408,12 +413,16 @@ export function createHooksRequestHandler(
sendJson(res, 400, { ok: false, error: sessionKey.error });
return true;
}
const targetAgentId = resolveHookTargetAgentId(hooksConfig, mapped.action.agentId);
const runId = dispatchAgentHook({
message: mapped.action.message,
name: mapped.action.name ?? "Hook",
agentId: resolveHookTargetAgentId(hooksConfig, mapped.action.agentId),
agentId: targetAgentId,
wakeMode: mapped.action.wakeMode,
sessionKey: sessionKey.value,
sessionKey: normalizeHookDispatchSessionKey({
sessionKey: sessionKey.value,
targetAgentId,
}),
deliver: resolveHookDeliver(mapped.action.deliver),
channel,
to: mapped.action.to,

View File

@@ -299,6 +299,48 @@ describe("gateway server hooks", () => {
});
});
test("normalizes duplicate target-agent prefixes before isolated dispatch", async () => {
testState.hooksConfig = {
enabled: true,
token: "hook-secret",
allowRequestSessionKey: true,
allowedSessionKeyPrefixes: ["hook:", "agent:"],
};
testState.agentsConfig = {
list: [{ id: "main", default: true }, { id: "hooks" }],
};
await withGatewayServer(async ({ port }) => {
cronIsolatedRun.mockClear();
cronIsolatedRun.mockResolvedValueOnce({
status: "ok",
summary: "done",
});
const resAgent = await fetch(`http://127.0.0.1:${port}/hooks/agent`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: "Bearer hook-secret",
},
body: JSON.stringify({
message: "Do it",
name: "Email",
agentId: "hooks",
sessionKey: "agent:hooks:slack:channel:c123",
}),
});
expect(resAgent.status).toBe(202);
await waitForSystemEvent();
const routedCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as
| { sessionKey?: string; job?: { agentId?: string } }
| undefined;
expect(routedCall?.job?.agentId).toBe("hooks");
expect(routedCall?.sessionKey).toBe("slack:channel:c123");
drainSystemEvents(resolveMainKey());
});
});
test("enforces hooks.allowedAgentIds for explicit agent routing", async () => {
testState.hooksConfig = {
enabled: true,

View File

@@ -7,7 +7,11 @@ import type { CronJob } from "../../cron/types.js";
import { requestHeartbeatNow } from "../../infra/heartbeat-wake.js";
import { enqueueSystemEvent } from "../../infra/system-events.js";
import type { createSubsystemLogger } from "../../logging/subsystem.js";
import type { HookAgentDispatchPayload, HooksConfigResolved } from "../hooks.js";
import {
normalizeHookDispatchSessionKey,
type HookAgentDispatchPayload,
type HooksConfigResolved,
} from "../hooks.js";
import { createHooksRequestHandler } from "../server-http.js";
type SubsystemLogger = ReturnType<typeof createSubsystemLogger>;
@@ -30,7 +34,10 @@ export function createGatewayHooksRequestHandler(params: {
};
const dispatchAgentHook = (value: HookAgentDispatchPayload) => {
const sessionKey = value.sessionKey.trim();
const sessionKey = normalizeHookDispatchSessionKey({
sessionKey: value.sessionKey,
targetAgentId: value.agentId,
});
const mainSessionKey = resolveMainSessionKeyFromConfig();
const jobId = randomUUID();
const now = Date.now();

View File

@@ -295,6 +295,32 @@ describe("loadOpenClawPlugins", () => {
expect(Object.keys(registry.gatewayHandlers)).toContain("allowed.ping");
});
it("loads plugins when source and root differ only by realpath alias", () => {
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins";
const plugin = writePlugin({
id: "alias-safe",
body: `export default { id: "alias-safe", register() {} };`,
});
const realRoot = fs.realpathSync(plugin.dir);
if (realRoot === plugin.dir) {
return;
}
const registry = loadOpenClawPlugins({
cache: false,
workspaceDir: plugin.dir,
config: {
plugins: {
load: { paths: [plugin.file] },
allow: ["alias-safe"],
},
},
});
const loaded = registry.plugins.find((entry) => entry.id === "alias-safe");
expect(loaded?.status).toBe("loaded");
});
it("denylist disables plugins even if allowed", () => {
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins";
const plugin = writePlugin({

View File

@@ -530,6 +530,10 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
absolutePath: candidate.source,
rootPath: pluginRoot,
boundaryLabel: "plugin root",
// Discovery stores rootDir as realpath but source may still be a lexical alias
// (e.g. /var/... vs /private/var/... on macOS). Canonical boundary checks
// still enforce containment; skip lexical pre-check to avoid false escapes.
skipLexicalRootCheck: true,
});
if (!opened.ok) {
record.status = "error";

View File

@@ -4,7 +4,11 @@ import {
getSubagentDepth,
isCronSessionKey,
} from "../sessions/session-key-utils.js";
import { classifySessionKeyShape } from "./session-key.js";
import {
classifySessionKeyShape,
parseAgentSessionKey,
toAgentStoreSessionKey,
} from "./session-key.js";
describe("classifySessionKeyShape", () => {
it("classifies empty keys as missing", () => {
@@ -93,3 +97,21 @@ describe("deriveSessionChatType", () => {
expect(deriveSessionChatType("")).toBe("unknown");
});
});
describe("session key canonicalization", () => {
it("parses agent keys case-insensitively and returns lowercase tokens", () => {
expect(parseAgentSessionKey("AGENT:Main:Hook:Webhook:42")).toEqual({
agentId: "main",
rest: "hook:webhook:42",
});
});
it("does not double-prefix already-qualified agent keys", () => {
expect(
toAgentStoreSessionKey({
agentId: "main",
requestKey: "agent:main:main",
}),
).toBe("agent:main:main");
});
});

View File

@@ -49,16 +49,17 @@ export function toAgentStoreSessionKey(params: {
mainKey?: string | undefined;
}): string {
const raw = (params.requestKey ?? "").trim();
if (!raw || raw === DEFAULT_MAIN_KEY) {
if (!raw || raw.toLowerCase() === DEFAULT_MAIN_KEY) {
return buildAgentMainSessionKey({ agentId: params.agentId, mainKey: params.mainKey });
}
const parsed = parseAgentSessionKey(raw);
if (parsed) {
return `agent:${parsed.agentId}:${parsed.rest}`;
}
const lowered = raw.toLowerCase();
if (lowered.startsWith("agent:")) {
return lowered;
}
if (lowered.startsWith("subagent:")) {
return `agent:${normalizeAgentId(params.agentId)}:${lowered}`;
}
return `agent:${normalizeAgentId(params.agentId)}:${lowered}`;
}

View File

@@ -5,10 +5,14 @@ export type ParsedAgentSessionKey = {
export type SessionKeyChatType = "direct" | "group" | "channel" | "unknown";
/**
* Parse agent-scoped session keys in a canonical, case-insensitive way.
* Returned values are normalized to lowercase for stable comparisons/routing.
*/
export function parseAgentSessionKey(
sessionKey: string | undefined | null,
): ParsedAgentSessionKey | null {
const raw = (sessionKey ?? "").trim();
const raw = (sessionKey ?? "").trim().toLowerCase();
if (!raw) {
return null;
}