mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 19:00:45 +00:00
Gate Slack startup user allowlist resolution [AI] (#77898)
* fix: gate slack user allowlist resolution * addressing codex review * addressing ci * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
ceaa56fb12
commit
b895c6d939
@@ -111,6 +111,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Gate Slack startup user allowlist resolution [AI]. (#77898) Thanks @pgondhi987.
|
||||
- OpenAI/Codex: suppress stale `openai-codex` GPT-5.1/5.2/5.3 model refs that ChatGPT/Codex OAuth accounts now reject, keeping model lists, config validation, and forward-compat resolution on current 5.4/5.5 routes. Fixes #67158. Thanks @drpau.
|
||||
- Google Meet/Voice Call: wait longer before playing PIN-derived Twilio DTMF for Meet dial-in prompts and retire stale delegated phone sessions instead of reusing completed calls.
|
||||
- PDF/Codex: include extraction-fallback instructions for `openai-codex/*` PDF tool requests so Codex Responses receives its required system prompt. Fixes #77872. Thanks @anyech.
|
||||
|
||||
@@ -19,6 +19,9 @@ type SlackTestState = {
|
||||
reactionRemoveMock: Mock<(...args: unknown[]) => unknown>;
|
||||
readAllowFromStoreMock: Mock<(...args: unknown[]) => Promise<unknown>>;
|
||||
upsertPairingRequestMock: Mock<(...args: unknown[]) => Promise<unknown>>;
|
||||
resolveSlackUserAllowlistMock: Mock<
|
||||
(params: { entries: string[] }) => Promise<Array<{ input: string; resolved: boolean }>>
|
||||
>;
|
||||
};
|
||||
|
||||
const slackTestState: SlackTestState = vi.hoisted(() => ({
|
||||
@@ -31,6 +34,7 @@ const slackTestState: SlackTestState = vi.hoisted(() => ({
|
||||
reactionRemoveMock: vi.fn(),
|
||||
readAllowFromStoreMock: vi.fn(),
|
||||
upsertPairingRequestMock: vi.fn(),
|
||||
resolveSlackUserAllowlistMock: vi.fn(),
|
||||
}));
|
||||
|
||||
export const getSlackTestState = (): SlackTestState => slackTestState;
|
||||
@@ -199,6 +203,11 @@ export function resetSlackTestState(config: Record<string, unknown> = defaultSla
|
||||
code: "PAIRCODE",
|
||||
created: true,
|
||||
});
|
||||
slackTestState.resolveSlackUserAllowlistMock
|
||||
.mockReset()
|
||||
.mockImplementation(async ({ entries }) =>
|
||||
entries.map((input) => ({ input, resolved: false })),
|
||||
);
|
||||
getSlackHandlers()?.clear();
|
||||
}
|
||||
|
||||
@@ -240,8 +249,8 @@ vi.mock("./resolve-channels.js", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./resolve-users.js", () => ({
|
||||
resolveSlackUserAllowlist: async ({ entries }: { entries: string[] }) =>
|
||||
entries.map((input) => ({ input, resolved: false })),
|
||||
resolveSlackUserAllowlist: (params: { entries: string[] }) =>
|
||||
slackTestState.resolveSlackUserAllowlistMock(params),
|
||||
}));
|
||||
|
||||
vi.mock("./monitor/send.runtime.js", () => {
|
||||
|
||||
@@ -1,6 +1,21 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
flush,
|
||||
getSlackHandlerOrThrow,
|
||||
getSlackTestState,
|
||||
resetSlackTestState,
|
||||
startSlackMonitor,
|
||||
stopSlackMonitor,
|
||||
} from "../monitor.test-helpers.js";
|
||||
import { formatSlackChannelResolved, formatSlackUserResolved } from "./provider-support.js";
|
||||
|
||||
const { monitorSlackProvider } = await import("./provider.js");
|
||||
const slackTestState = getSlackTestState();
|
||||
|
||||
beforeEach(() => {
|
||||
resetSlackTestState();
|
||||
});
|
||||
|
||||
describe("slack allowlist log formatting", () => {
|
||||
it("prints channel names alongside ids", () => {
|
||||
expect(
|
||||
@@ -24,3 +39,99 @@ describe("slack allowlist log formatting", () => {
|
||||
).toBe("U090HHQ029J→steipete (id:U090HHQ029J)");
|
||||
});
|
||||
});
|
||||
|
||||
describe("slack startup user allowlist resolution", () => {
|
||||
it("skips user entry resolution when name matching is not enabled", async () => {
|
||||
resetSlackTestState({
|
||||
messages: {
|
||||
responsePrefix: "PFX",
|
||||
},
|
||||
channels: {
|
||||
slack: {
|
||||
enabled: true,
|
||||
dmPolicy: "allowlist",
|
||||
allowFrom: ["<@U123GLOBAL>", "@global-user"],
|
||||
channels: {
|
||||
C123: {
|
||||
enabled: true,
|
||||
requireMention: false,
|
||||
users: ["<@U123CHANNEL>", "@channel-user"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
slackTestState.replyMock.mockResolvedValue({ text: "ok" });
|
||||
|
||||
const monitor = startSlackMonitor(monitorSlackProvider);
|
||||
try {
|
||||
const handler = await getSlackHandlerOrThrow("message");
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
expect(slackTestState.resolveSlackUserAllowlistMock).not.toHaveBeenCalled();
|
||||
|
||||
await handler({
|
||||
event: {
|
||||
type: "message",
|
||||
user: "U123GLOBAL",
|
||||
text: "hello",
|
||||
ts: "100.000",
|
||||
channel: "D123",
|
||||
channel_type: "im",
|
||||
},
|
||||
});
|
||||
expect(slackTestState.replyMock).toHaveBeenCalledTimes(1);
|
||||
|
||||
slackTestState.replyMock.mockClear();
|
||||
await handler({
|
||||
event: {
|
||||
type: "message",
|
||||
user: "U123CHANNEL",
|
||||
text: "hello",
|
||||
ts: "101.000",
|
||||
channel: "C123",
|
||||
channel_type: "channel",
|
||||
},
|
||||
});
|
||||
expect(slackTestState.replyMock).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
await stopSlackMonitor(monitor);
|
||||
}
|
||||
});
|
||||
|
||||
it("resolves user entries when name matching is enabled", async () => {
|
||||
resetSlackTestState({
|
||||
channels: {
|
||||
slack: {
|
||||
enabled: true,
|
||||
dangerouslyAllowNameMatching: true,
|
||||
dmPolicy: "allowlist",
|
||||
allowFrom: ["@global-user"],
|
||||
channels: {
|
||||
C123: { users: ["@channel-user"] },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const monitor = startSlackMonitor(monitorSlackProvider);
|
||||
try {
|
||||
await getSlackHandlerOrThrow("message");
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
expect(slackTestState.resolveSlackUserAllowlistMock).toHaveBeenCalledTimes(2);
|
||||
expect(slackTestState.resolveSlackUserAllowlistMock).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.objectContaining({ entries: ["@global-user"] }),
|
||||
);
|
||||
expect(slackTestState.resolveSlackUserAllowlistMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.objectContaining({ entries: ["@channel-user"] }),
|
||||
);
|
||||
} finally {
|
||||
await stopSlackMonitor(monitor);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,7 +32,7 @@ import { isSlackExecApprovalClientEnabled } from "../exec-approvals.js";
|
||||
import { normalizeSlackWebhookPath, registerSlackHttpHandler } from "../http/index.js";
|
||||
import { SLACK_TEXT_LIMIT } from "../limits.js";
|
||||
import { resolveSlackChannelAllowlist } from "../resolve-channels.js";
|
||||
import { resolveSlackUserAllowlist } from "../resolve-users.js";
|
||||
import { resolveSlackUserAllowlist, type SlackUserResolution } from "../resolve-users.js";
|
||||
import { resolveSlackAppToken, resolveSlackBotToken } from "../token.js";
|
||||
import { normalizeAllowList } from "./allow-list.js";
|
||||
import { resolveSlackSlashCommandConfig } from "./commands.js";
|
||||
@@ -85,6 +85,33 @@ async function getSlackBoltInterop(): Promise<SlackBoltResolvedExports> {
|
||||
const SLACK_WEBHOOK_MAX_BODY_BYTES = 1024 * 1024;
|
||||
const SLACK_WEBHOOK_BODY_TIMEOUT_MS = 30_000;
|
||||
|
||||
function resolveStableSlackUserIdEntry(raw: string): string | undefined {
|
||||
const trimmed = raw.trim();
|
||||
if (!trimmed) {
|
||||
return undefined;
|
||||
}
|
||||
const mention = /^<@([A-Z][A-Z0-9]+)>$/i.exec(trimmed);
|
||||
if (mention) {
|
||||
return mention[1]?.toUpperCase();
|
||||
}
|
||||
const prefixed = /^(?:slack:|user:)([A-Z][A-Z0-9]+)$/i.exec(trimmed);
|
||||
if (prefixed) {
|
||||
return prefixed[1]?.toUpperCase();
|
||||
}
|
||||
return /^[UW][A-Z0-9]+$/i.test(trimmed) ? trimmed.toUpperCase() : undefined;
|
||||
}
|
||||
|
||||
function resolveStableSlackUserAllowlistEntries(entries: string[]): SlackUserResolution[] {
|
||||
const resolved: SlackUserResolution[] = [];
|
||||
for (const input of entries) {
|
||||
const id = resolveStableSlackUserIdEntry(input);
|
||||
if (id) {
|
||||
resolved.push({ input, resolved: true, id });
|
||||
}
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
|
||||
export function formatSlackSocketReconnectMessage(params: {
|
||||
event: string;
|
||||
attempt: number;
|
||||
@@ -209,6 +236,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
|
||||
const threadInheritParent = slackCfg.thread?.inheritParent ?? false;
|
||||
const threadRequireExplicitMention = slackCfg.thread?.requireExplicitMention ?? false;
|
||||
const slashCommand = resolveSlackSlashCommandConfig(opts.slashCommand ?? slackCfg.slashCommand);
|
||||
const allowNameMatching = isDangerousNameMatchingEnabled(slackCfg);
|
||||
const textLimit = resolveTextChunkLimit(cfg, "slack", account.accountId, {
|
||||
fallbackLimit: SLACK_TEXT_LIMIT,
|
||||
});
|
||||
@@ -301,7 +329,7 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
|
||||
dmEnabled,
|
||||
dmPolicy,
|
||||
allowFrom,
|
||||
allowNameMatching: isDangerousNameMatchingEnabled(slackCfg),
|
||||
allowNameMatching,
|
||||
groupDmEnabled,
|
||||
groupDmChannels,
|
||||
defaultRequireMention: slackCfg.requireMention,
|
||||
@@ -404,24 +432,36 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
|
||||
|
||||
const allowEntries = normalizeStringEntries(allowFrom).filter((entry) => entry !== "*");
|
||||
if (allowEntries.length > 0) {
|
||||
try {
|
||||
const resolvedUsers = await resolveSlackUserAllowlist({
|
||||
token: resolveToken,
|
||||
entries: allowEntries,
|
||||
const stableResolvedUsers = resolveStableSlackUserAllowlistEntries(allowEntries);
|
||||
if (stableResolvedUsers.length > 0) {
|
||||
const { mapping, additions } = buildAllowlistResolutionSummary(stableResolvedUsers, {
|
||||
formatResolved: formatSlackUserResolved,
|
||||
});
|
||||
const { mapping, unresolved, additions } = buildAllowlistResolutionSummary(
|
||||
resolvedUsers,
|
||||
{
|
||||
formatResolved: formatSlackUserResolved,
|
||||
},
|
||||
);
|
||||
allowFrom = mergeAllowlist({ existing: allowFrom, additions });
|
||||
ctx.allowFrom = normalizeAllowList(allowFrom);
|
||||
summarizeMapping("slack users", mapping, unresolved, runtime);
|
||||
} catch (err) {
|
||||
runtime.log?.(
|
||||
`slack user resolve failed; using config entries. ${formatUnknownError(err)}`,
|
||||
);
|
||||
summarizeMapping("slack users", mapping, [], runtime);
|
||||
}
|
||||
|
||||
if (allowNameMatching) {
|
||||
try {
|
||||
const resolvedUsers = await resolveSlackUserAllowlist({
|
||||
token: resolveToken,
|
||||
entries: allowEntries,
|
||||
});
|
||||
const { mapping, unresolved, additions } = buildAllowlistResolutionSummary(
|
||||
resolvedUsers,
|
||||
{
|
||||
formatResolved: formatSlackUserResolved,
|
||||
},
|
||||
);
|
||||
allowFrom = mergeAllowlist({ existing: allowFrom, additions });
|
||||
ctx.allowFrom = normalizeAllowList(allowFrom);
|
||||
summarizeMapping("slack users", mapping, unresolved, runtime);
|
||||
} catch (err) {
|
||||
runtime.log?.(
|
||||
`slack user resolve failed; using config entries. ${formatUnknownError(err)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -432,29 +472,47 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) {
|
||||
}
|
||||
|
||||
if (userEntries.size > 0) {
|
||||
try {
|
||||
const resolvedUsers = await resolveSlackUserAllowlist({
|
||||
token: resolveToken,
|
||||
entries: Array.from(userEntries),
|
||||
const stableResolvedUsers = resolveStableSlackUserAllowlistEntries(
|
||||
Array.from(userEntries),
|
||||
);
|
||||
if (stableResolvedUsers.length > 0) {
|
||||
const { resolvedMap, mapping } = buildAllowlistResolutionSummary(stableResolvedUsers, {
|
||||
formatResolved: formatSlackUserResolved,
|
||||
});
|
||||
const { resolvedMap, mapping, unresolved } = buildAllowlistResolutionSummary(
|
||||
resolvedUsers,
|
||||
{
|
||||
formatResolved: formatSlackUserResolved,
|
||||
},
|
||||
);
|
||||
|
||||
const nextChannels = patchAllowlistUsersInConfigEntries({
|
||||
entries: channelsConfig,
|
||||
resolvedMap,
|
||||
});
|
||||
channelsConfig = nextChannels;
|
||||
ctx.channelsConfig = nextChannels;
|
||||
summarizeMapping("slack channel users", mapping, unresolved, runtime);
|
||||
} catch (err) {
|
||||
runtime.log?.(
|
||||
`slack channel user resolve failed; using config entries. ${formatUnknownError(err)}`,
|
||||
);
|
||||
summarizeMapping("slack channel users", mapping, [], runtime);
|
||||
}
|
||||
|
||||
if (allowNameMatching) {
|
||||
try {
|
||||
const resolvedUsers = await resolveSlackUserAllowlist({
|
||||
token: resolveToken,
|
||||
entries: Array.from(userEntries),
|
||||
});
|
||||
const { resolvedMap, mapping, unresolved } = buildAllowlistResolutionSummary(
|
||||
resolvedUsers,
|
||||
{
|
||||
formatResolved: formatSlackUserResolved,
|
||||
},
|
||||
);
|
||||
|
||||
const nextChannels = patchAllowlistUsersInConfigEntries({
|
||||
entries: channelsConfig,
|
||||
resolvedMap,
|
||||
});
|
||||
channelsConfig = nextChannels;
|
||||
ctx.channelsConfig = nextChannels;
|
||||
summarizeMapping("slack channel users", mapping, unresolved, runtime);
|
||||
} catch (err) {
|
||||
runtime.log?.(
|
||||
`slack channel user resolve failed; using config entries. ${formatUnknownError(err)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user