mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 11:00:42 +00:00
fix(plugins): tighten register rollback
This commit is contained in:
@@ -10,10 +10,12 @@ const hoisted = vi.hoisted(() => {
|
||||
const sendMessageMock = vi.fn();
|
||||
const cancelSessionMock = vi.fn();
|
||||
const killSubagentRunAdminMock = vi.fn();
|
||||
const resolveConfiguredSecretInputStringMock = vi.fn();
|
||||
return {
|
||||
sendMessageMock,
|
||||
cancelSessionMock,
|
||||
killSubagentRunAdminMock,
|
||||
resolveConfiguredSecretInputStringMock,
|
||||
};
|
||||
});
|
||||
|
||||
@@ -31,6 +33,17 @@ vi.mock("../../../src/agents/subagent-control.js", () => ({
|
||||
killSubagentRunAdmin: (params: unknown) => hoisted.killSubagentRunAdminMock(params),
|
||||
}));
|
||||
|
||||
vi.mock("../runtime-api.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../runtime-api.js")>();
|
||||
hoisted.resolveConfiguredSecretInputStringMock.mockImplementation(
|
||||
actual.resolveConfiguredSecretInputString,
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
resolveConfiguredSecretInputString: hoisted.resolveConfiguredSecretInputStringMock,
|
||||
};
|
||||
});
|
||||
|
||||
type MockIncomingMessage = IncomingMessage & {
|
||||
destroyed?: boolean;
|
||||
destroy: () => MockIncomingMessage;
|
||||
@@ -58,7 +71,7 @@ function createJsonRequest(params: {
|
||||
return req;
|
||||
}) as MockIncomingMessage["destroy"];
|
||||
|
||||
void Promise.resolve().then(() => {
|
||||
setImmediate(() => {
|
||||
req.emit("data", Buffer.from(JSON.stringify(params.body), "utf8"));
|
||||
req.emit("end");
|
||||
});
|
||||
@@ -95,6 +108,17 @@ function createHandler(): {
|
||||
};
|
||||
}
|
||||
|
||||
function createHandlerWithTarget(
|
||||
target: TaskFlowWebhookTarget,
|
||||
cfg: OpenClawConfig = {} as OpenClawConfig,
|
||||
): ReturnType<typeof createTaskFlowWebhookRequestHandler> {
|
||||
const targetsByPath = new Map<string, TaskFlowWebhookTarget[]>([[target.path, [target]]]);
|
||||
return createTaskFlowWebhookRequestHandler({
|
||||
cfg,
|
||||
targetsByPath,
|
||||
});
|
||||
}
|
||||
|
||||
async function dispatchJsonRequest(params: {
|
||||
handler: ReturnType<typeof createTaskFlowWebhookRequestHandler>;
|
||||
path: string;
|
||||
@@ -136,6 +160,47 @@ describe("createTaskFlowWebhookRequestHandler", () => {
|
||||
expect(target.taskFlow.list()).toEqual([]);
|
||||
});
|
||||
|
||||
it("caches SecretRef resolution across requests for the same route", async () => {
|
||||
const runtime = createRuntimeTaskFlow();
|
||||
const target: TaskFlowWebhookTarget = {
|
||||
routeId: "cached",
|
||||
path: "/plugins/webhooks/cached",
|
||||
secretInput: {
|
||||
source: "env",
|
||||
provider: "default",
|
||||
id: "OPENCLAW_WEBHOOK_SECRET",
|
||||
},
|
||||
secretConfigPath: "plugins.entries.webhooks.routes.cached.secret",
|
||||
defaultControllerId: "webhooks/cached",
|
||||
taskFlow: runtime.bindSession({
|
||||
sessionKey: "agent:main:webhook-cached",
|
||||
}),
|
||||
};
|
||||
hoisted.resolveConfiguredSecretInputStringMock.mockResolvedValue({ value: "shared-secret" });
|
||||
const handler = createHandlerWithTarget(target);
|
||||
|
||||
const first = await dispatchJsonRequest({
|
||||
handler,
|
||||
path: target.path,
|
||||
secret: "shared-secret",
|
||||
body: {
|
||||
action: "list_flows",
|
||||
},
|
||||
});
|
||||
const second = await dispatchJsonRequest({
|
||||
handler,
|
||||
path: target.path,
|
||||
secret: "shared-secret",
|
||||
body: {
|
||||
action: "list_flows",
|
||||
},
|
||||
});
|
||||
|
||||
expect(first.statusCode).toBe(200);
|
||||
expect(second.statusCode).toBe(200);
|
||||
expect(hoisted.resolveConfiguredSecretInputStringMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("creates flows through the bound session and scrubs owner metadata from responses", async () => {
|
||||
const { handler, target, secret } = createHandler();
|
||||
const res = await dispatchJsonRequest({
|
||||
|
||||
@@ -667,6 +667,7 @@ export function createTaskFlowWebhookRequestHandler(params: {
|
||||
targetsByPath: Map<string, TaskFlowWebhookTarget[]>;
|
||||
inFlightLimiter?: WebhookInFlightLimiter;
|
||||
}): (req: IncomingMessage, res: ServerResponse) => Promise<boolean> {
|
||||
const secretByTarget = new WeakMap<TaskFlowWebhookTarget, Promise<string | undefined>>();
|
||||
const rateLimiter = createFixedWindowRateLimiter({
|
||||
windowMs: WEBHOOK_RATE_LIMIT_DEFAULTS.windowMs,
|
||||
maxRequests: WEBHOOK_RATE_LIMIT_DEFAULTS.maxRequests,
|
||||
@@ -678,6 +679,20 @@ export function createTaskFlowWebhookRequestHandler(params: {
|
||||
maxInFlightPerKey: WEBHOOK_IN_FLIGHT_DEFAULTS.maxInFlightPerKey,
|
||||
maxTrackedKeys: WEBHOOK_IN_FLIGHT_DEFAULTS.maxTrackedKeys,
|
||||
});
|
||||
const resolveTargetSecret = (target: TaskFlowWebhookTarget): Promise<string | undefined> => {
|
||||
const cached = secretByTarget.get(target);
|
||||
if (cached) {
|
||||
return cached;
|
||||
}
|
||||
const pending = resolveConfiguredSecretInputString({
|
||||
config: params.cfg,
|
||||
env: process.env,
|
||||
value: target.secretInput,
|
||||
path: target.secretConfigPath,
|
||||
}).then((resolved) => resolved.value);
|
||||
secretByTarget.set(target, pending);
|
||||
return pending;
|
||||
};
|
||||
|
||||
return async (req: IncomingMessage, res: ServerResponse): Promise<boolean> => {
|
||||
return await withResolvedWebhookRequestPipeline({
|
||||
@@ -708,14 +723,9 @@ export function createTaskFlowWebhookRequestHandler(params: {
|
||||
if (presentedSecret.length === 0) {
|
||||
return false;
|
||||
}
|
||||
const resolvedSecret = await resolveConfiguredSecretInputString({
|
||||
config: params.cfg,
|
||||
env: process.env,
|
||||
value: candidate.secretInput,
|
||||
path: candidate.secretConfigPath,
|
||||
});
|
||||
const resolvedSecret = await resolveTargetSecret(candidate);
|
||||
return Boolean(
|
||||
resolvedSecret.value && timingSafeEquals(resolvedSecret.value, presentedSecret),
|
||||
resolvedSecret && timingSafeEquals(resolvedSecret, presentedSecret),
|
||||
);
|
||||
},
|
||||
});
|
||||
|
||||
@@ -1795,6 +1795,18 @@ module.exports = { id: "throws-after-import", register() {} };`,
|
||||
description: "Fail me",
|
||||
handler: async () => ({ text: "nope" }),
|
||||
});
|
||||
api.registerReload({
|
||||
onConfigReload: async () => {},
|
||||
});
|
||||
api.registerNodeHostCommand({
|
||||
command: "failme",
|
||||
description: "failme",
|
||||
run: async () => ({ ok: true }),
|
||||
});
|
||||
api.registerSecurityAuditCollector({
|
||||
id: "failme",
|
||||
collect: async () => [],
|
||||
});
|
||||
api.registerInteractiveHandler({
|
||||
channel: "slack",
|
||||
namespace: "failme",
|
||||
@@ -1831,6 +1843,9 @@ module.exports = { id: "throws-after-import", register() {} };`,
|
||||
);
|
||||
expect(getRegisteredEventKeys()).toEqual([]);
|
||||
expect(getPluginCommandSpecs()).toEqual([]);
|
||||
expect(registry.reloads).toEqual([]);
|
||||
expect(registry.nodeHostCommands).toEqual([]);
|
||||
expect(registry.securityAuditCollectors).toEqual([]);
|
||||
expect(resolvePluginInteractiveNamespaceMatch("slack", "failme:payload")).toBeNull();
|
||||
expect(getContextEngineFactory("failme-context")).toBeUndefined();
|
||||
expect(listContextEngineIds()).not.toContain("failme-context");
|
||||
|
||||
@@ -2209,6 +2209,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
hookPolicy: entry?.hooks,
|
||||
registrationMode,
|
||||
});
|
||||
const registrySnapshot = snapshotPluginRegistry(registry);
|
||||
const previousAgentHarnesses = listRegisteredAgentHarnesses();
|
||||
const previousCompactionProviders = listRegisteredCompactionProviders();
|
||||
const previousMemoryEmbeddingProviders = listRegisteredMemoryEmbeddingProviders();
|
||||
@@ -2217,7 +2218,6 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
||||
const previousMemoryCorpusSupplements = listMemoryCorpusSupplements();
|
||||
const previousMemoryPromptSupplements = listMemoryPromptSupplements();
|
||||
const previousMemoryRuntime = getMemoryRuntime();
|
||||
const registrySnapshot = snapshotPluginRegistry(registry);
|
||||
|
||||
try {
|
||||
runPluginRegisterSync(register, api);
|
||||
|
||||
Reference in New Issue
Block a user