Skip to content

Commit e5205a4

Browse files
Pick-catPick-cat
authored
fix(gateway): scope tools.effective global agent lookup
Pass the requested agent into tools.effective global-session loading while preserving non-global session ownership guards, with handler and gateway regression coverage.
1 parent 45f261f commit e5205a4

4 files changed

Lines changed: 357 additions & 9 deletions

File tree

‎src/gateway/server-methods/tools-effective.global-agent.integration.test.ts‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
// Integration proof for tools.effective global sessions scoped to non-default agents.
2+
import path from "node:path";
3+
import { beforeEach, describe, expect, it, vi } from "vitest";
4+
import { installGatewayTestHooks, testState, writeSessionStore } from "../test-helpers.js";
5+
import { getGatewayConfigModule, sessionStoreEntry } from "../test/server-sessions.test-helpers.js";
6+
import { testing, toolsEffectiveHandlers } from "./tools-effective.js";
7+
8+
const inventoryMocks = vi.hoisted(() => ({
9+
resolveEffectiveToolInventory: vi.fn(
10+
(params: { agentId: string; modelProvider?: string; modelId?: string }) => ({
11+
agentId: params.agentId,
12+
profile: "coding",
13+
groups: [
14+
{
15+
id: "core",
16+
label: "Built-in tools",
17+
source: "core",
18+
tools: [
19+
{
20+
id: "exec",
21+
label: "Exec",
22+
description: "Run shell commands",
23+
source: "core",
24+
},
25+
],
26+
},
27+
],
28+
modelProvider: params.modelProvider,
29+
modelId: params.modelId,
30+
}),
31+
),
32+
resolveEffectiveToolInventoryRuntimeModelContext: vi.fn(() => ({
33+
modelApi: "openai-responses",
34+
runtimeModel: {
35+
id: "work-model",
36+
name: "Work model",
37+
provider: "openai",
38+
api: "openai-responses",
39+
baseUrl: "https://api.openai.com/v1",
40+
},
41+
})),
42+
}));
43+
44+
vi.mock("./tools-effective.runtime.js", async (importOriginal) => {
45+
const actual = await importOriginal<typeof import("./tools-effective.runtime.js")>();
46+
return {
47+
...actual,
48+
resolveEffectiveToolInventory: inventoryMocks.resolveEffectiveToolInventory,
49+
resolveEffectiveToolInventoryRuntimeModelContext:
50+
inventoryMocks.resolveEffectiveToolInventoryRuntimeModelContext,
51+
peekSessionMcpRuntime: vi.fn(() => undefined),
52+
resolveSessionMcpConfigSummary: vi.fn(() => ({ fingerprint: "mcp:0", serverNames: [] })),
53+
buildBundleMcpToolsFromCatalog: vi.fn(() => []),
54+
applyFinalEffectiveToolPolicy: vi.fn(
55+
(params: { bundledTools: unknown[] }) => params.bundledTools,
56+
),
57+
getActivePluginRegistryVersion: vi.fn(() => 1),
58+
getActivePluginChannelRegistryVersion: vi.fn(() => 1),
59+
};
60+
});
61+
62+
installGatewayTestHooks();
63+
64+
describe("tools.effective global agent integration", () => {
65+
let mainStorePath = "";
66+
let workStorePath = "";
67+
let getRuntimeConfig: Awaited<ReturnType<typeof getGatewayConfigModule>>["getRuntimeConfig"];
68+
69+
async function seedSelectedGlobalStores() {
70+
const stateDir = process.env.OPENCLAW_STATE_DIR;
71+
if (!stateDir) {
72+
throw new Error("OPENCLAW_STATE_DIR is required");
73+
}
74+
const dir = path.join(stateDir, "session-stores", `tools-effective-${Date.now()}`);
75+
const storeTemplate = path.join(dir, "{agentId}", "sessions.json");
76+
testState.sessionStorePath = storeTemplate;
77+
testState.sessionConfig = { scope: "global" };
78+
testState.agentsConfig = { list: [{ id: "main", default: true }, { id: "work" }] };
79+
mainStorePath = storeTemplate.replace("{agentId}", "main");
80+
workStorePath = storeTemplate.replace("{agentId}", "work");
81+
const configModule = await getGatewayConfigModule();
82+
configModule.clearRuntimeConfigSnapshot();
83+
configModule.clearConfigCache();
84+
getRuntimeConfig = configModule.getRuntimeConfig;
85+
}
86+
87+
beforeEach(async () => {
88+
testing.resetToolsEffectiveCacheForTest();
89+
vi.clearAllMocks();
90+
await seedSelectedGlobalStores();
91+
});
92+
93+
it("resolves tools.effective for global session scoped to a non-default agent store", async () => {
94+
await writeSessionStore({
95+
storePath: mainStorePath,
96+
entries: {
97+
global: sessionStoreEntry("sess-main-global", {
98+
modelProvider: "openai",
99+
model: "main-model",
100+
}),
101+
},
102+
});
103+
await writeSessionStore({
104+
storePath: workStorePath,
105+
agentId: "work",
106+
entries: {
107+
global: sessionStoreEntry("sess-work-global", {
108+
modelProvider: "openai",
109+
model: "work-model",
110+
}),
111+
},
112+
});
113+
114+
const respond = vi.fn();
115+
await toolsEffectiveHandlers["tools.effective"]({
116+
params: { sessionKey: "global", agentId: "work" },
117+
respond: respond as never,
118+
context: { getRuntimeConfig } as never,
119+
client: null,
120+
req: { type: "req", id: "req-tools-effective-global", method: "tools.effective" },
121+
isWebchatConnect: () => false,
122+
});
123+
124+
const call = respond.mock.calls[0] as [boolean, { agentId?: string }?, unknown?] | undefined;
125+
expect(call?.[0]).toBe(true);
126+
expect(call?.[1]?.agentId).toBe("work");
127+
expect(inventoryMocks.resolveEffectiveToolInventory).toHaveBeenCalledWith(
128+
expect.objectContaining({
129+
agentId: "work",
130+
sessionKey: "global",
131+
modelProvider: "openai",
132+
modelId: "work-model",
133+
}),
134+
);
135+
});
136+
137+
// Negative control on the real session-resolution path: a non-global key owned
138+
// by `main` must keep rejecting a mismatched configured agent. Before the
139+
// ownership-narrowing fix the requested agent overrode session-agent resolution
140+
// here, so this request would have succeeded under `work`.
141+
it("rejects a mismatched configured agent for a non-global session key", async () => {
142+
await seedNonGlobalMainStore();
143+
144+
await writeSessionStore({
145+
storePath: mainStorePath,
146+
entries: {
147+
"agent:main:abc": sessionStoreEntry("sess-main-agent", {
148+
modelProvider: "openai",
149+
model: "main-model",
150+
}),
151+
},
152+
});
153+
154+
const respond = vi.fn();
155+
await toolsEffectiveHandlers["tools.effective"]({
156+
params: { sessionKey: "agent:main:abc", agentId: "work" },
157+
respond: respond as never,
158+
context: { getRuntimeConfig } as never,
159+
client: null,
160+
req: { type: "req", id: "req-tools-effective-mismatch", method: "tools.effective" },
161+
isWebchatConnect: () => false,
162+
});
163+
164+
const call = respond.mock.calls[0] as
165+
| [boolean, unknown?, { code: number; message: string }?]
166+
| undefined;
167+
expect(call?.[0]).toBe(false);
168+
expect(call?.[2]?.message).toBe('agent id "work" does not match session agent "main"');
169+
expect(inventoryMocks.resolveEffectiveToolInventory).not.toHaveBeenCalled();
170+
});
171+
172+
async function seedNonGlobalMainStore() {
173+
const stateDir = process.env.OPENCLAW_STATE_DIR;
174+
if (!stateDir) {
175+
throw new Error("OPENCLAW_STATE_DIR is required");
176+
}
177+
const dir = path.join(stateDir, "session-stores", `tools-effective-nonglobal-${Date.now()}`);
178+
const storeTemplate = path.join(dir, "{agentId}", "sessions.json");
179+
testState.sessionStorePath = storeTemplate;
180+
testState.sessionConfig = undefined;
181+
testState.agentsConfig = { list: [{ id: "main", default: true }, { id: "work" }] };
182+
mainStorePath = storeTemplate.replace("{agentId}", "main");
183+
const configModule = await getGatewayConfigModule();
184+
configModule.clearRuntimeConfigSnapshot();
185+
configModule.clearConfigCache();
186+
getRuntimeConfig = configModule.getRuntimeConfig;
187+
}
188+
});

‎src/gateway/server-methods/tools-effective.test.ts‎

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -574,20 +574,82 @@ describe("tools.effective handler", () => {
574574
expect(firstRespondCall(respond)?.[0]).toBe(true);
575575
});
576576

577-
it("rejects agent ids that do not match the session agent", async () => {
577+
it("rejects unknown agent ids before loading the session", async () => {
578578
const { respond, invoke } = createInvokeParams({
579579
sessionKey: "main:abc",
580580
agentId: "other",
581581
});
582+
// `other` is not configured, so the handler rejects before reaching
583+
// loadSessionEntry; no session mock is queued here on purpose.
584+
await invoke();
585+
expectInvalidResponse(respond, 'unknown agent id "other"');
586+
expect(runtimeMocks.loadSessionEntry).not.toHaveBeenCalled();
587+
});
588+
589+
it("loads global sessions with the requested agent id before matching session agent", async () => {
590+
runtimeMocks.listAgentIds.mockReturnValueOnce(["main", "work"]);
591+
runtimeMocks.loadSessionEntry.mockImplementationOnce(
592+
() =>
593+
({
594+
cfg: {},
595+
canonicalKey: "global",
596+
entry: {
597+
sessionId: "session-work-global",
598+
updatedAt: 1,
599+
modelProvider: "openai",
600+
model: "gpt-4.1",
601+
},
602+
storePath: "/tmp/work/sessions.json",
603+
store: {},
604+
storeKeys: ["global"],
605+
}) as never,
606+
);
607+
runtimeMocks.resolveSessionAgentId.mockReturnValueOnce("work");
608+
runtimeMocks.resolveAgentDir.mockReturnValueOnce("/tmp/agents/work/agent");
609+
runtimeMocks.resolveAgentWorkspaceDir.mockReturnValueOnce("/tmp/workspace-work");
610+
runtimeMocks.resolveEffectiveToolInventory.mockReturnValueOnce(makeCoreInventory());
611+
612+
const { respond, invoke } = createInvokeParams({
613+
sessionKey: "global",
614+
agentId: "work",
615+
});
616+
await invoke();
617+
618+
expect(runtimeMocks.loadSessionEntry).toHaveBeenCalledWith("global", { agentId: "work" });
619+
expect(runtimeMocks.resolveSessionAgentId).toHaveBeenCalledWith(
620+
expect.objectContaining({
621+
agentId: "work",
622+
}),
623+
);
624+
const call = firstRespondCall(respond);
625+
expect(call?.[0]).toBe(true);
626+
expect(resolveEffectiveToolInventoryArg()?.agentId).toBe("work");
627+
expect(runtimeMocks.resolveAgentDir).toHaveBeenCalledWith({}, "work");
628+
});
629+
630+
it("does not let a requested agent override ownership of a non-global session key", async () => {
631+
runtimeMocks.listAgentIds.mockReturnValueOnce(["main", "work"]);
582632
runtimeMocks.loadSessionEntry.mockReturnValueOnce({
583633
cfg: {},
584-
canonicalKey: "main:abc",
585-
entry: {
586-
sessionId: "session-1",
587-
updatedAt: 1,
588-
},
634+
canonicalKey: "agent:main:abc",
635+
entry: { sessionId: "session-main", updatedAt: 1 },
589636
} as never);
637+
// Persisted owner of the non-global key.
638+
runtimeMocks.resolveSessionAgentId.mockReturnValueOnce("main");
639+
640+
const { respond, invoke } = createInvokeParams({
641+
sessionKey: "agent:main:abc",
642+
agentId: "work",
643+
});
590644
await invoke();
591-
expectInvalidResponse(respond, 'unknown agent id "other"');
645+
646+
// Wiring guard: for a non-global key the requested agent must NOT be forwarded
647+
// as the session-agent override, otherwise the real resolver would prefer
648+
// "work" and silently pass the mismatch check below.
649+
expect(runtimeMocks.resolveSessionAgentId).toHaveBeenLastCalledWith(
650+
expect.not.objectContaining({ agentId: expect.anything() }),
651+
);
652+
expectInvalidResponse(respond, 'agent id "work" does not match session agent "main"');
653+
expect(runtimeMocks.resolveEffectiveToolInventory).not.toHaveBeenCalled();
592654
});
593655
});

‎src/gateway/server-methods/tools-effective.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,10 @@ function resolveTrustedToolsEffectiveContext(params: {
459459
}) {
460460
// The effective tools request is read-only but security-sensitive. Derive
461461
// routing/account/model context from the persisted session, not client params.
462-
const loaded = loadSessionEntry(params.sessionKey);
462+
const loaded = loadSessionEntry(
463+
params.sessionKey,
464+
params.requestedAgentId ? { agentId: params.requestedAgentId } : undefined,
465+
);
463466
if (!loaded.entry) {
464467
params.respond(
465468
false,
@@ -469,9 +472,18 @@ function resolveTrustedToolsEffectiveContext(params: {
469472
return null;
470473
}
471474

475+
// Only a canonical `global` key may adopt the client-requested agent: global
476+
// stores are shared, so the requested agent selects which agent's global store
477+
// to read. Non-global keys encode their owning agent, so the requested agent
478+
// must stay subject to the mismatch guard below instead of overriding session
479+
// ownership — otherwise `{ sessionKey: "agent:main:x", agentId: "work" }` would
480+
// resolve under `work` and silently bypass the guard.
481+
const canonicalKey = loaded.canonicalKey ?? params.sessionKey;
482+
const allowRequestedAgentOverride = canonicalKey === "global" && Boolean(params.requestedAgentId);
472483
const sessionAgentId = resolveSessionAgentId({
473-
sessionKey: loaded.canonicalKey ?? params.sessionKey,
484+
sessionKey: canonicalKey,
474485
config: loaded.cfg,
486+
...(allowRequestedAgentOverride ? { agentId: params.requestedAgentId } : {}),
475487
});
476488
if (params.requestedAgentId && params.requestedAgentId !== sessionAgentId) {
477489
params.respond(

‎src/gateway/server.tools-effective.global-agent-gateway.test.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* Live Gateway integration test for the tools.effective agent-ownership guard.
3+
*
4+
* Drives the real Gateway over a WebSocket RPC client against an on-disk session
5+
* store and config, proving a non-global session rejects a mismatched configured
6+
* agent before any inventory work (the security-sensitive ownership path).
7+
*
8+
* The positive global-session case is covered at the handler-integration layer
9+
* (`tools-effective.global-agent.integration.test.ts`); the live sessions
10+
* harness deliberately stubs the bundled MCP inventory runtime, so a successful
11+
* tools.effective inventory cannot resolve here without materializing bundled
12+
* plugin runtime.
13+
*/
14+
import fs from "node:fs/promises";
15+
import path from "node:path";
16+
import { expect, test } from "vitest";
17+
import { ErrorCodes } from "../../packages/gateway-protocol/src/index.js";
18+
import { rpcReq, testState } from "./test-helpers.js";
19+
import {
20+
getGatewayConfigModule,
21+
setupGatewaySessionsTestHarness,
22+
} from "./test/server-sessions.test-helpers.js";
23+
24+
const { openClient } = setupGatewaySessionsTestHarness();
25+
26+
test("tools.effective rejects a mismatched configured agent for a non-global session key", async () => {
27+
const reset = await configureNonGlobalMainSession();
28+
const { ws } = await openClient();
29+
try {
30+
const res = await rpcReq<{ agentId?: string }>(ws, "tools.effective", {
31+
sessionKey: "agent:main:abc",
32+
agentId: "work",
33+
});
34+
expect(res.ok).toBe(false);
35+
expect(res.error).toEqual({
36+
code: ErrorCodes.INVALID_REQUEST,
37+
message: 'agent id "work" does not match session agent "main"',
38+
});
39+
} finally {
40+
ws.close();
41+
await reset();
42+
}
43+
});
44+
45+
/**
46+
* Writes a non-global config (agents main+work, default scope) plus a single
47+
* `agent:main:abc` session owned by `main`, and returns a cleanup callback.
48+
*/
49+
async function configureNonGlobalMainSession(): Promise<() => Promise<void>> {
50+
const configPath = process.env.OPENCLAW_CONFIG_PATH;
51+
const stateDir = process.env.OPENCLAW_STATE_DIR;
52+
if (!configPath || !stateDir) {
53+
throw new Error("OPENCLAW_CONFIG_PATH and OPENCLAW_STATE_DIR are required");
54+
}
55+
const dir = path.join(stateDir, "session-stores", `tools-effective-nonglobal-${Date.now()}`);
56+
const storePath = path.join(dir, "sessions.json");
57+
testState.sessionStorePath = storePath;
58+
testState.sessionConfig = undefined;
59+
await fs.mkdir(dir, { recursive: true });
60+
await fs.writeFile(
61+
storePath,
62+
`${JSON.stringify({ "agent:main:abc": { sessionId: "sess-main-agent", updatedAt: 1 } }, null, 2)}\n`,
63+
"utf-8",
64+
);
65+
await fs.writeFile(
66+
configPath,
67+
`${JSON.stringify(
68+
{
69+
agents: { list: [{ id: "main", default: true }, { id: "work" }] },
70+
session: { store: storePath },
71+
},
72+
null,
73+
2,
74+
)}\n`,
75+
"utf-8",
76+
);
77+
const { clearConfigCache, clearRuntimeConfigSnapshot } = await getGatewayConfigModule();
78+
clearRuntimeConfigSnapshot();
79+
clearConfigCache();
80+
return async () => {
81+
testState.sessionStorePath = undefined;
82+
await fs.writeFile(configPath, "{}\n", "utf-8");
83+
clearRuntimeConfigSnapshot();
84+
clearConfigCache();
85+
};
86+
}

0 commit comments

Comments
 (0)