fix(slack): resolve user IDs to DM channels before files.uploadV2 (#23773)

When a bare Slack user ID (U-prefix) is passed as the send target
without an explicit `user:` prefix, `parseSlackTarget` classifies it as
kind="channel".  `resolveChannelId` then passes it through to callers
without calling `conversations.open`.

This works for `chat.postMessage` (which tolerates user IDs), but
`files.uploadV2` delegates to `completeUploadExternal` which validates
`channel_id` against `^[CGDZ][A-Z0-9]{8,}$` — rejecting U-prefixed
IDs with `invalid_arguments`.

Fix: detect U-prefixed IDs in `resolveChannelId` regardless of the
parsed `kind`, and always resolve them via `conversations.open` to
obtain the DM channel ID (D-prefix).

Includes test coverage for bare, prefixed, and mention-style user ID
targets with file uploads, plus a channel-target negative case.
This commit is contained in:
Luis Conde
2026-02-22 14:04:53 -04:00
committed by GitHub
parent 568973e5ac
commit af9881b9c5
3 changed files with 132 additions and 1 deletions

View File

@@ -51,6 +51,7 @@ Docs: https://docs.openclaw.ai
- Memory/Embeddings: apply configured remote-base host pinning (`allowedHostnames`) across OpenAI/Voyage/Gemini embedding requests to keep private/self-hosted endpoints working without cross-host drift. (#18198) Thanks @ianpcook.
- Memory/Batch: route OpenAI/Voyage/Gemini batch upload/create/status/download requests through the same guarded HTTP path for consistent SSRF policy enforcement.
- Install/Discord Voice: make `@discordjs/opus` an optional dependency so `openclaw` install/update no longer hard-fails when native Opus builds fail, while keeping `opusscript` as the runtime fallback decoder for Discord voice flows. (#23737, #23733, #23703)
- Slack/Upload: resolve bare user IDs (U-prefix) to DM channel IDs via `conversations.open` before calling `files.uploadV2`, which rejects non-channel IDs. `chat.postMessage` tolerates user IDs directly, but `files.uploadV2``completeUploadExternal` validates `channel_id` against `^[CGDZ][A-Z0-9]{8,}$`, causing `invalid_arguments` when agents reply with media to DM conversations.
- Signal/RPC: guard malformed Signal RPC JSON responses with a clear status-scoped error and add regression coverage for invalid JSON responses. (#22995) Thanks @adhitShet.
- Gateway/Subagents: guard gateway and subagent session-key/message trim paths against undefined inputs to prevent early `Cannot read properties of undefined (reading 'trim')` crashes during subagent spawn and wait flows.
- Agents/Workspace: guard `resolveUserPath` against undefined/null input to prevent `Cannot read properties of undefined (reading 'trim')` crashes when workspace paths are missing in embedded runner flows.

View File

@@ -166,7 +166,14 @@ async function resolveChannelId(
client: WebClient,
recipient: SlackRecipient,
): Promise<{ channelId: string; isDm?: boolean }> {
if (recipient.kind === "channel") {
// Bare Slack user IDs (U-prefix) may arrive with kind="channel" when the
// target string had no explicit prefix (parseSlackTarget defaults bare IDs
// to "channel"). chat.postMessage tolerates user IDs directly, but
// files.uploadV2 → completeUploadExternal validates channel_id against
// ^[CGDZ][A-Z0-9]{8,}$ and rejects U-prefixed IDs. Always resolve user
// IDs via conversations.open to obtain the DM channel ID.
const isUserId = recipient.kind === "user" || /^U[A-Z0-9]+$/i.test(recipient.id);
if (!isUserId) {
return { channelId: recipient.id };
}
const response = await client.conversations.open({ users: recipient.id });

View File

@@ -0,0 +1,123 @@
import type { WebClient } from "@slack/web-api";
import { describe, expect, it, vi } from "vitest";
// --- Module mocks (must precede dynamic import) ---
vi.mock("../config/config.js", () => ({
loadConfig: () => ({}),
}));
vi.mock("./accounts.js", () => ({
resolveSlackAccount: () => ({
accountId: "default",
botToken: "xoxb-test",
botTokenSource: "config",
config: {},
}),
}));
vi.mock("../web/media.js", () => ({
loadWebMedia: vi.fn(async () => ({
buffer: Buffer.from("fake-image"),
contentType: "image/png",
kind: "image",
fileName: "screenshot.png",
})),
}));
const { sendMessageSlack } = await import("./send.js");
type UploadTestClient = WebClient & {
conversations: { open: ReturnType<typeof vi.fn> };
chat: { postMessage: ReturnType<typeof vi.fn> };
files: { uploadV2: ReturnType<typeof vi.fn> };
};
function createUploadTestClient(): UploadTestClient {
return {
conversations: {
open: vi.fn(async () => ({ channel: { id: "D99RESOLVED" } })),
},
chat: {
postMessage: vi.fn(async () => ({ ts: "171234.567" })),
},
files: {
uploadV2: vi.fn(async () => ({ files: [{ id: "F001" }] })),
},
} as unknown as UploadTestClient;
}
describe("sendMessageSlack file upload with user IDs", () => {
it("resolves bare user ID to DM channel before files.uploadV2", async () => {
const client = createUploadTestClient();
// Bare user ID — parseSlackTarget classifies this as kind="channel"
await sendMessageSlack("U2ZH3MFSR", "screenshot", {
token: "xoxb-test",
client,
mediaUrl: "/tmp/screenshot.png",
});
// Should call conversations.open to resolve user ID → DM channel
expect(client.conversations.open).toHaveBeenCalledWith({
users: "U2ZH3MFSR",
});
// files.uploadV2 should receive the resolved DM channel ID, not the user ID
expect(client.files.uploadV2).toHaveBeenCalledWith(
expect.objectContaining({
channel_id: "D99RESOLVED",
filename: "screenshot.png",
}),
);
});
it("resolves prefixed user ID to DM channel before files.uploadV2", async () => {
const client = createUploadTestClient();
await sendMessageSlack("user:UABC123", "image", {
token: "xoxb-test",
client,
mediaUrl: "/tmp/photo.png",
});
expect(client.conversations.open).toHaveBeenCalledWith({
users: "UABC123",
});
expect(client.files.uploadV2).toHaveBeenCalledWith(
expect.objectContaining({ channel_id: "D99RESOLVED" }),
);
});
it("sends file directly to channel without conversations.open", async () => {
const client = createUploadTestClient();
await sendMessageSlack("channel:C123CHAN", "chart", {
token: "xoxb-test",
client,
mediaUrl: "/tmp/chart.png",
});
expect(client.conversations.open).not.toHaveBeenCalled();
expect(client.files.uploadV2).toHaveBeenCalledWith(
expect.objectContaining({ channel_id: "C123CHAN" }),
);
});
it("resolves mention-style user ID before file upload", async () => {
const client = createUploadTestClient();
await sendMessageSlack("<@U777TEST>", "report", {
token: "xoxb-test",
client,
mediaUrl: "/tmp/report.png",
});
expect(client.conversations.open).toHaveBeenCalledWith({
users: "U777TEST",
});
expect(client.files.uploadV2).toHaveBeenCalledWith(
expect.objectContaining({ channel_id: "D99RESOLVED" }),
);
});
});