mirror of
https://github.com/moltbot/moltbot.git
synced 2026-05-13 23:56:07 +00:00
fix(msteams): gate startup user allowlist resolution [AI] (#79003)
* fix: gate msteams user allowlist name resolution * addressing codex review * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
8fc53e7937
commit
c1edfafa3e
@@ -172,6 +172,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- fix(msteams): gate startup user allowlist resolution [AI]. (#79003) Thanks @pgondhi987.
|
||||
- Harden macOS shell wrapper allowlist parsing [AI]. (#78518) Thanks @pgondhi987.
|
||||
- Gateway/macOS: `openclaw gateway stop` now uses `launchctl bootout` by default instead of unconditionally calling `launchctl disable`, so KeepAlive auto-recovery still works after unexpected crashes; use the new `--disable` flag to opt into the persistent-disable behavior when a manual stop should survive reboots. Fixes #77934. Thanks @bmoran1022.
|
||||
- Gateway/macOS: `repairLaunchAgentBootstrap` no longer kickstarts an already-running LaunchAgent, preventing unnecessary service restarts and session disconnects when repair runs against a healthy gateway. Fixes #77428. Thanks @ramitrkar-hash.
|
||||
|
||||
@@ -21,8 +21,11 @@ const expressControl = vi.hoisted(() => ({
|
||||
}>,
|
||||
}));
|
||||
|
||||
const isDangerousNameMatchingEnabled = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../runtime-api.js", () => ({
|
||||
DEFAULT_WEBHOOK_MAX_BODY_BYTES: 1024 * 1024,
|
||||
isDangerousNameMatchingEnabled,
|
||||
normalizeSecretInputString: (value: unknown) =>
|
||||
typeof value === "string" && value.trim() ? value.trim() : undefined,
|
||||
hasConfiguredSecretInput: (value: unknown) =>
|
||||
@@ -115,12 +118,17 @@ const loadMSTeamsSdkWithAuth = vi.hoisted(() =>
|
||||
);
|
||||
|
||||
vi.mock("./monitor-handler.js", () => ({
|
||||
registerMSTeamsHandlers: () => registerMSTeamsHandlers(),
|
||||
registerMSTeamsHandlers: (...args: unknown[]) => registerMSTeamsHandlers(...args),
|
||||
}));
|
||||
|
||||
const resolveAllowlistMocks = vi.hoisted(() => ({
|
||||
resolveMSTeamsChannelAllowlist: vi.fn(async () => []),
|
||||
resolveMSTeamsUserAllowlist: vi.fn(async () => []),
|
||||
}));
|
||||
|
||||
vi.mock("./resolve-allowlist.js", () => ({
|
||||
resolveMSTeamsChannelAllowlist: vi.fn(async () => []),
|
||||
resolveMSTeamsUserAllowlist: vi.fn(async () => []),
|
||||
resolveMSTeamsChannelAllowlist: resolveAllowlistMocks.resolveMSTeamsChannelAllowlist,
|
||||
resolveMSTeamsUserAllowlist: resolveAllowlistMocks.resolveMSTeamsUserAllowlist,
|
||||
}));
|
||||
|
||||
vi.mock("./sdk.js", () => ({
|
||||
@@ -192,6 +200,9 @@ describe("monitorMSTeamsProvider lifecycle", () => {
|
||||
vi.clearAllMocks();
|
||||
expressControl.mode.value = "listening";
|
||||
expressControl.apps.length = 0;
|
||||
isDangerousNameMatchingEnabled.mockReset().mockReturnValue(false);
|
||||
resolveAllowlistMocks.resolveMSTeamsChannelAllowlist.mockReset().mockResolvedValue([]);
|
||||
resolveAllowlistMocks.resolveMSTeamsUserAllowlist.mockReset().mockResolvedValue([]);
|
||||
jwtValidate.mockReset().mockResolvedValue(true);
|
||||
});
|
||||
|
||||
@@ -277,4 +288,106 @@ describe("monitorMSTeamsProvider lifecycle", () => {
|
||||
abort.abort();
|
||||
await task;
|
||||
});
|
||||
|
||||
it("does not resolve user allowlists by display name unless name matching is enabled", async () => {
|
||||
const abort = new AbortController();
|
||||
const cfg = createConfig(0);
|
||||
cfg.channels!.msteams = {
|
||||
...cfg.channels!.msteams!,
|
||||
allowFrom: ["Alice", "user:40a1a0ed-4ff2-4164-a219-55518990c197"],
|
||||
groupAllowFrom: ["Bob", "msteams:user:50a1a0ed-4ff2-4164-a219-55518990c198"],
|
||||
teams: {
|
||||
Product: {
|
||||
channels: {
|
||||
Roadmap: {},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
resolveAllowlistMocks.resolveMSTeamsChannelAllowlist.mockResolvedValueOnce([
|
||||
{
|
||||
input: "Product/Roadmap",
|
||||
resolved: true,
|
||||
teamId: "team-id",
|
||||
channelId: "channel-id",
|
||||
},
|
||||
]);
|
||||
|
||||
const task = monitorMSTeamsProvider({
|
||||
cfg,
|
||||
runtime: createRuntime(),
|
||||
abortSignal: abort.signal,
|
||||
conversationStore: createStores().conversationStore,
|
||||
pollStore: createStores().pollStore,
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(registerMSTeamsHandlers).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
expect(resolveAllowlistMocks.resolveMSTeamsUserAllowlist).not.toHaveBeenCalled();
|
||||
expect(resolveAllowlistMocks.resolveMSTeamsChannelAllowlist).toHaveBeenCalledWith({
|
||||
cfg,
|
||||
entries: ["Product/Roadmap"],
|
||||
});
|
||||
|
||||
const registeredCfg = registerMSTeamsHandlers.mock.calls[0]?.[1] as { cfg: OpenClawConfig };
|
||||
expect(registeredCfg.cfg.channels?.msteams?.allowFrom).toEqual([
|
||||
"Alice",
|
||||
"user:40a1a0ed-4ff2-4164-a219-55518990c197",
|
||||
"40a1a0ed-4ff2-4164-a219-55518990c197",
|
||||
]);
|
||||
expect(registeredCfg.cfg.channels?.msteams?.groupAllowFrom).toEqual([
|
||||
"Bob",
|
||||
"msteams:user:50a1a0ed-4ff2-4164-a219-55518990c198",
|
||||
"50a1a0ed-4ff2-4164-a219-55518990c198",
|
||||
]);
|
||||
|
||||
abort.abort();
|
||||
await task;
|
||||
});
|
||||
|
||||
it("resolves user allowlists when name matching is enabled", async () => {
|
||||
isDangerousNameMatchingEnabled.mockReturnValue(true);
|
||||
resolveAllowlistMocks.resolveMSTeamsUserAllowlist
|
||||
.mockResolvedValueOnce([{ input: "Alice", resolved: true, id: "alice-aad" }])
|
||||
.mockResolvedValueOnce([{ input: "Bob", resolved: true, id: "bob-aad" }]);
|
||||
|
||||
const abort = new AbortController();
|
||||
const cfg = createConfig(0);
|
||||
cfg.channels!.msteams = {
|
||||
...cfg.channels!.msteams!,
|
||||
dangerouslyAllowNameMatching: true,
|
||||
allowFrom: ["Alice"],
|
||||
groupAllowFrom: ["Bob"],
|
||||
};
|
||||
|
||||
const task = monitorMSTeamsProvider({
|
||||
cfg,
|
||||
runtime: createRuntime(),
|
||||
abortSignal: abort.signal,
|
||||
conversationStore: createStores().conversationStore,
|
||||
pollStore: createStores().pollStore,
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(registerMSTeamsHandlers).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
expect(resolveAllowlistMocks.resolveMSTeamsUserAllowlist).toHaveBeenNthCalledWith(1, {
|
||||
cfg,
|
||||
entries: ["Alice"],
|
||||
});
|
||||
expect(resolveAllowlistMocks.resolveMSTeamsUserAllowlist).toHaveBeenNthCalledWith(2, {
|
||||
cfg,
|
||||
entries: ["Bob"],
|
||||
});
|
||||
|
||||
const registeredCfg = registerMSTeamsHandlers.mock.calls[0]?.[1] as { cfg: OpenClawConfig };
|
||||
expect(registeredCfg.cfg.channels?.msteams?.allowFrom).toEqual(["Alice", "alice-aad"]);
|
||||
expect(registeredCfg.cfg.channels?.msteams?.groupAllowFrom).toEqual(["Bob", "bob-aad"]);
|
||||
|
||||
abort.abort();
|
||||
await task;
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { Request, Response } from "express";
|
||||
import {
|
||||
DEFAULT_WEBHOOK_MAX_BODY_BYTES,
|
||||
isDangerousNameMatchingEnabled,
|
||||
keepHttpServerTaskAlive,
|
||||
mergeAllowlist,
|
||||
summarizeMapping,
|
||||
@@ -73,12 +74,20 @@ export async function monitorMSTeamsProvider(
|
||||
let allowFrom = msteamsCfg.allowFrom;
|
||||
let groupAllowFrom = msteamsCfg.groupAllowFrom;
|
||||
let teamsConfig = msteamsCfg.teams;
|
||||
const allowNameMatching = isDangerousNameMatchingEnabled(msteamsCfg);
|
||||
|
||||
const cleanAllowEntry = (entry: string) =>
|
||||
entry
|
||||
.replace(/^(msteams|teams):/i, "")
|
||||
.replace(/^user:/i, "")
|
||||
.trim();
|
||||
const isStableUserId = (entry: string) => /^[0-9a-fA-F-]{16,}$/.test(entry);
|
||||
const cleanAllowEntries = (entries?: string[]) =>
|
||||
entries?.map((entry) => cleanAllowEntry(entry)).filter((entry) => entry && entry !== "*") ?? [];
|
||||
const mergeStableUserIds = (entries?: string[]) => {
|
||||
const additions = cleanAllowEntries(entries).filter((entry) => isStableUserId(entry));
|
||||
return additions.length > 0 ? mergeAllowlist({ existing: entries, additions }) : entries;
|
||||
};
|
||||
|
||||
const resolveAllowlistUsers = async (label: string, entries: string[]) => {
|
||||
if (entries.length === 0) {
|
||||
@@ -102,21 +111,26 @@ export async function monitorMSTeamsProvider(
|
||||
};
|
||||
|
||||
try {
|
||||
const allowEntries =
|
||||
allowFrom?.map((entry) => cleanAllowEntry(entry)).filter((entry) => entry && entry !== "*") ??
|
||||
[];
|
||||
if (allowEntries.length > 0) {
|
||||
const { additions } = await resolveAllowlistUsers("msteams users", allowEntries);
|
||||
allowFrom = mergeAllowlist({ existing: allowFrom, additions });
|
||||
allowFrom = mergeStableUserIds(allowFrom);
|
||||
if (Array.isArray(groupAllowFrom) && groupAllowFrom.length > 0) {
|
||||
groupAllowFrom = mergeStableUserIds(groupAllowFrom);
|
||||
}
|
||||
|
||||
if (Array.isArray(groupAllowFrom) && groupAllowFrom.length > 0) {
|
||||
const groupEntries = groupAllowFrom
|
||||
.map((entry) => cleanAllowEntry(entry))
|
||||
.filter((entry) => entry && entry !== "*");
|
||||
if (groupEntries.length > 0) {
|
||||
const { additions } = await resolveAllowlistUsers("msteams group users", groupEntries);
|
||||
groupAllowFrom = mergeAllowlist({ existing: groupAllowFrom, additions });
|
||||
if (allowNameMatching) {
|
||||
const allowEntries = cleanAllowEntries(allowFrom).filter((entry) => !isStableUserId(entry));
|
||||
if (allowEntries.length > 0) {
|
||||
const { additions } = await resolveAllowlistUsers("msteams users", allowEntries);
|
||||
allowFrom = mergeAllowlist({ existing: allowFrom, additions });
|
||||
}
|
||||
|
||||
if (Array.isArray(groupAllowFrom) && groupAllowFrom.length > 0) {
|
||||
const groupEntries = cleanAllowEntries(groupAllowFrom).filter(
|
||||
(entry) => !isStableUserId(entry),
|
||||
);
|
||||
if (groupEntries.length > 0) {
|
||||
const { additions } = await resolveAllowlistUsers("msteams group users", groupEntries);
|
||||
groupAllowFrom = mergeAllowlist({ existing: groupAllowFrom, additions });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user