fix: gate Teams media auth retries
This commit is contained in:
@@ -456,6 +456,7 @@ Key settings (see `/gateway/configuration` for shared channel patterns):
|
|||||||
- `channels.msteams.textChunkLimit`: outbound text chunk size.
|
- `channels.msteams.textChunkLimit`: outbound text chunk size.
|
||||||
- `channels.msteams.chunkMode`: `length` (default) or `newline` to split on blank lines (paragraph boundaries) before length chunking.
|
- `channels.msteams.chunkMode`: `length` (default) or `newline` to split on blank lines (paragraph boundaries) before length chunking.
|
||||||
- `channels.msteams.mediaAllowHosts`: allowlist for inbound attachment hosts (defaults to Microsoft/Teams domains).
|
- `channels.msteams.mediaAllowHosts`: allowlist for inbound attachment hosts (defaults to Microsoft/Teams domains).
|
||||||
|
- `channels.msteams.mediaAuthAllowHosts`: allowlist for attaching Authorization headers on media retries (defaults to Graph + Bot Framework hosts).
|
||||||
- `channels.msteams.requireMention`: require @mention in channels/groups (default true).
|
- `channels.msteams.requireMention`: require @mention in channels/groups (default true).
|
||||||
- `channels.msteams.replyStyle`: `thread | top-level` (see [Reply Style](#reply-style-threads-vs-posts)).
|
- `channels.msteams.replyStyle`: `thread | top-level` (see [Reply Style](#reply-style-threads-vs-posts)).
|
||||||
- `channels.msteams.teams.<teamId>.replyStyle`: per-team override.
|
- `channels.msteams.teams.<teamId>.replyStyle`: per-team override.
|
||||||
@@ -518,6 +519,7 @@ Teams recently introduced two channel UI styles over the same underlying data mo
|
|||||||
|
|
||||||
Without Graph permissions, channel messages with images will be received as text-only (the image content is not accessible to the bot).
|
Without Graph permissions, channel messages with images will be received as text-only (the image content is not accessible to the bot).
|
||||||
By default, OpenClaw only downloads media from Microsoft/Teams hostnames. Override with `channels.msteams.mediaAllowHosts` (use `["*"]` to allow any host).
|
By default, OpenClaw only downloads media from Microsoft/Teams hostnames. Override with `channels.msteams.mediaAllowHosts` (use `["*"]` to allow any host).
|
||||||
|
Authorization headers are only attached for hosts in `channels.msteams.mediaAuthAllowHosts` (defaults to Graph + Bot Framework hosts). Keep this list strict (avoid multi-tenant suffixes).
|
||||||
|
|
||||||
## Sending files in group chats
|
## Sending files in group chats
|
||||||
|
|
||||||
|
|||||||
@@ -241,6 +241,7 @@ describe("msteams attachments", () => {
|
|||||||
maxBytes: 1024 * 1024,
|
maxBytes: 1024 * 1024,
|
||||||
tokenProvider: { getAccessToken: vi.fn(async () => "token") },
|
tokenProvider: { getAccessToken: vi.fn(async () => "token") },
|
||||||
allowHosts: ["x"],
|
allowHosts: ["x"],
|
||||||
|
authAllowHosts: ["x"],
|
||||||
fetchFn: fetchMock as unknown as typeof fetch,
|
fetchFn: fetchMock as unknown as typeof fetch,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -249,6 +250,41 @@ describe("msteams attachments", () => {
|
|||||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("skips auth retries when the host is not in auth allowlist", async () => {
|
||||||
|
const { downloadMSTeamsAttachments } = await load();
|
||||||
|
const tokenProvider = { getAccessToken: vi.fn(async () => "token") };
|
||||||
|
const fetchMock = vi.fn(async (_url: string, opts?: RequestInit) => {
|
||||||
|
const hasAuth = Boolean(
|
||||||
|
opts &&
|
||||||
|
typeof opts === "object" &&
|
||||||
|
"headers" in opts &&
|
||||||
|
(opts.headers as Record<string, string>)?.Authorization,
|
||||||
|
);
|
||||||
|
if (!hasAuth) {
|
||||||
|
return new Response("forbidden", { status: 403 });
|
||||||
|
}
|
||||||
|
return new Response(Buffer.from("png"), {
|
||||||
|
status: 200,
|
||||||
|
headers: { "content-type": "image/png" },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
const media = await downloadMSTeamsAttachments({
|
||||||
|
attachments: [
|
||||||
|
{ contentType: "image/png", contentUrl: "https://attacker.azureedge.net/img" },
|
||||||
|
],
|
||||||
|
maxBytes: 1024 * 1024,
|
||||||
|
tokenProvider,
|
||||||
|
allowHosts: ["azureedge.net"],
|
||||||
|
authAllowHosts: ["graph.microsoft.com"],
|
||||||
|
fetchFn: fetchMock as unknown as typeof fetch,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(media).toHaveLength(0);
|
||||||
|
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||||
|
expect(tokenProvider.getAccessToken).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("skips urls outside the allowlist", async () => {
|
it("skips urls outside the allowlist", async () => {
|
||||||
const { downloadMSTeamsAttachments } = await load();
|
const { downloadMSTeamsAttachments } = await load();
|
||||||
const fetchMock = vi.fn();
|
const fetchMock = vi.fn();
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import {
|
|||||||
isRecord,
|
isRecord,
|
||||||
isUrlAllowed,
|
isUrlAllowed,
|
||||||
normalizeContentType,
|
normalizeContentType,
|
||||||
|
resolveAuthAllowedHosts,
|
||||||
resolveAllowedHosts,
|
resolveAllowedHosts,
|
||||||
} from "./shared.js";
|
} from "./shared.js";
|
||||||
|
|
||||||
@@ -85,6 +86,8 @@ async function fetchWithAuthFallback(params: {
|
|||||||
url: string;
|
url: string;
|
||||||
tokenProvider?: MSTeamsAccessTokenProvider;
|
tokenProvider?: MSTeamsAccessTokenProvider;
|
||||||
fetchFn?: typeof fetch;
|
fetchFn?: typeof fetch;
|
||||||
|
allowHosts: string[];
|
||||||
|
authAllowHosts: string[];
|
||||||
}): Promise<Response> {
|
}): Promise<Response> {
|
||||||
const fetchFn = params.fetchFn ?? fetch;
|
const fetchFn = params.fetchFn ?? fetch;
|
||||||
const firstAttempt = await fetchFn(params.url);
|
const firstAttempt = await fetchFn(params.url);
|
||||||
@@ -97,6 +100,9 @@ async function fetchWithAuthFallback(params: {
|
|||||||
if (firstAttempt.status !== 401 && firstAttempt.status !== 403) {
|
if (firstAttempt.status !== 401 && firstAttempt.status !== 403) {
|
||||||
return firstAttempt;
|
return firstAttempt;
|
||||||
}
|
}
|
||||||
|
if (!isUrlAllowed(params.url, params.authAllowHosts)) {
|
||||||
|
return firstAttempt;
|
||||||
|
}
|
||||||
|
|
||||||
const scopes = scopeCandidatesForUrl(params.url);
|
const scopes = scopeCandidatesForUrl(params.url);
|
||||||
for (const scope of scopes) {
|
for (const scope of scopes) {
|
||||||
@@ -104,10 +110,30 @@ async function fetchWithAuthFallback(params: {
|
|||||||
const token = await params.tokenProvider.getAccessToken(scope);
|
const token = await params.tokenProvider.getAccessToken(scope);
|
||||||
const res = await fetchFn(params.url, {
|
const res = await fetchFn(params.url, {
|
||||||
headers: { Authorization: `Bearer ${token}` },
|
headers: { Authorization: `Bearer ${token}` },
|
||||||
|
redirect: "manual",
|
||||||
});
|
});
|
||||||
if (res.ok) {
|
if (res.ok) {
|
||||||
return res;
|
return res;
|
||||||
}
|
}
|
||||||
|
const redirectUrl = readRedirectUrl(params.url, res);
|
||||||
|
if (redirectUrl && isUrlAllowed(redirectUrl, params.allowHosts)) {
|
||||||
|
const redirectRes = await fetchFn(redirectUrl);
|
||||||
|
if (redirectRes.ok) {
|
||||||
|
return redirectRes;
|
||||||
|
}
|
||||||
|
if (
|
||||||
|
(redirectRes.status === 401 || redirectRes.status === 403) &&
|
||||||
|
isUrlAllowed(redirectUrl, params.authAllowHosts)
|
||||||
|
) {
|
||||||
|
const redirectAuthRes = await fetchFn(redirectUrl, {
|
||||||
|
headers: { Authorization: `Bearer ${token}` },
|
||||||
|
redirect: "manual",
|
||||||
|
});
|
||||||
|
if (redirectAuthRes.ok) {
|
||||||
|
return redirectAuthRes;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
} catch {
|
} catch {
|
||||||
// Try the next scope.
|
// Try the next scope.
|
||||||
}
|
}
|
||||||
@@ -116,6 +142,21 @@ async function fetchWithAuthFallback(params: {
|
|||||||
return firstAttempt;
|
return firstAttempt;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function readRedirectUrl(baseUrl: string, res: Response): string | null {
|
||||||
|
if (![301, 302, 303, 307, 308].includes(res.status)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
const location = res.headers.get("location");
|
||||||
|
if (!location) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
return new URL(location, baseUrl).toString();
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Download all file attachments from a Teams message (images, documents, etc.).
|
* Download all file attachments from a Teams message (images, documents, etc.).
|
||||||
* Renamed from downloadMSTeamsImageAttachments to support all file types.
|
* Renamed from downloadMSTeamsImageAttachments to support all file types.
|
||||||
@@ -125,6 +166,7 @@ export async function downloadMSTeamsAttachments(params: {
|
|||||||
maxBytes: number;
|
maxBytes: number;
|
||||||
tokenProvider?: MSTeamsAccessTokenProvider;
|
tokenProvider?: MSTeamsAccessTokenProvider;
|
||||||
allowHosts?: string[];
|
allowHosts?: string[];
|
||||||
|
authAllowHosts?: string[];
|
||||||
fetchFn?: typeof fetch;
|
fetchFn?: typeof fetch;
|
||||||
/** When true, embeds original filename in stored path for later extraction. */
|
/** When true, embeds original filename in stored path for later extraction. */
|
||||||
preserveFilenames?: boolean;
|
preserveFilenames?: boolean;
|
||||||
@@ -134,6 +176,7 @@ export async function downloadMSTeamsAttachments(params: {
|
|||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
const allowHosts = resolveAllowedHosts(params.allowHosts);
|
const allowHosts = resolveAllowedHosts(params.allowHosts);
|
||||||
|
const authAllowHosts = resolveAuthAllowedHosts(params.authAllowHosts);
|
||||||
|
|
||||||
// Download ANY downloadable attachment (not just images)
|
// Download ANY downloadable attachment (not just images)
|
||||||
const downloadable = list.filter(isDownloadableAttachment);
|
const downloadable = list.filter(isDownloadableAttachment);
|
||||||
@@ -199,6 +242,8 @@ export async function downloadMSTeamsAttachments(params: {
|
|||||||
url: candidate.url,
|
url: candidate.url,
|
||||||
tokenProvider: params.tokenProvider,
|
tokenProvider: params.tokenProvider,
|
||||||
fetchFn: params.fetchFn,
|
fetchFn: params.fetchFn,
|
||||||
|
allowHosts,
|
||||||
|
authAllowHosts,
|
||||||
});
|
});
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
continue;
|
continue;
|
||||||
|
|||||||
@@ -215,6 +215,7 @@ export async function downloadMSTeamsGraphMedia(params: {
|
|||||||
tokenProvider?: MSTeamsAccessTokenProvider;
|
tokenProvider?: MSTeamsAccessTokenProvider;
|
||||||
maxBytes: number;
|
maxBytes: number;
|
||||||
allowHosts?: string[];
|
allowHosts?: string[];
|
||||||
|
authAllowHosts?: string[];
|
||||||
fetchFn?: typeof fetch;
|
fetchFn?: typeof fetch;
|
||||||
/** When true, embeds original filename in stored path for later extraction. */
|
/** When true, embeds original filename in stored path for later extraction. */
|
||||||
preserveFilenames?: boolean;
|
preserveFilenames?: boolean;
|
||||||
@@ -336,6 +337,7 @@ export async function downloadMSTeamsGraphMedia(params: {
|
|||||||
maxBytes: params.maxBytes,
|
maxBytes: params.maxBytes,
|
||||||
tokenProvider: params.tokenProvider,
|
tokenProvider: params.tokenProvider,
|
||||||
allowHosts,
|
allowHosts,
|
||||||
|
authAllowHosts: params.authAllowHosts,
|
||||||
fetchFn: params.fetchFn,
|
fetchFn: params.fetchFn,
|
||||||
preserveFilenames: params.preserveFilenames,
|
preserveFilenames: params.preserveFilenames,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -48,6 +48,15 @@ export const DEFAULT_MEDIA_HOST_ALLOWLIST = [
|
|||||||
"microsoft.com",
|
"microsoft.com",
|
||||||
] as const;
|
] as const;
|
||||||
|
|
||||||
|
export const DEFAULT_MEDIA_AUTH_HOST_ALLOWLIST = [
|
||||||
|
"api.botframework.com",
|
||||||
|
"botframework.com",
|
||||||
|
"graph.microsoft.com",
|
||||||
|
"graph.microsoft.us",
|
||||||
|
"graph.microsoft.de",
|
||||||
|
"graph.microsoft.cn",
|
||||||
|
] as const;
|
||||||
|
|
||||||
export const GRAPH_ROOT = "https://graph.microsoft.com/v1.0";
|
export const GRAPH_ROOT = "https://graph.microsoft.com/v1.0";
|
||||||
|
|
||||||
export function isRecord(value: unknown): value is Record<string, unknown> {
|
export function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
@@ -250,6 +259,17 @@ export function resolveAllowedHosts(input?: string[]): string[] {
|
|||||||
return normalized;
|
return normalized;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function resolveAuthAllowedHosts(input?: string[]): string[] {
|
||||||
|
if (!Array.isArray(input) || input.length === 0) {
|
||||||
|
return DEFAULT_MEDIA_AUTH_HOST_ALLOWLIST.slice();
|
||||||
|
}
|
||||||
|
const normalized = input.map(normalizeAllowHost).filter(Boolean);
|
||||||
|
if (normalized.includes("*")) {
|
||||||
|
return ["*"];
|
||||||
|
}
|
||||||
|
return normalized;
|
||||||
|
}
|
||||||
|
|
||||||
function isHostAllowed(host: string, allowlist: string[]): boolean {
|
function isHostAllowed(host: string, allowlist: string[]): boolean {
|
||||||
if (allowlist.includes("*")) {
|
if (allowlist.includes("*")) {
|
||||||
return true;
|
return true;
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ export async function resolveMSTeamsInboundMedia(params: {
|
|||||||
htmlSummary?: MSTeamsHtmlAttachmentSummary;
|
htmlSummary?: MSTeamsHtmlAttachmentSummary;
|
||||||
maxBytes: number;
|
maxBytes: number;
|
||||||
allowHosts?: string[];
|
allowHosts?: string[];
|
||||||
|
authAllowHosts?: string[];
|
||||||
tokenProvider: MSTeamsAccessTokenProvider;
|
tokenProvider: MSTeamsAccessTokenProvider;
|
||||||
conversationType: string;
|
conversationType: string;
|
||||||
conversationId: string;
|
conversationId: string;
|
||||||
@@ -46,6 +47,7 @@ export async function resolveMSTeamsInboundMedia(params: {
|
|||||||
maxBytes,
|
maxBytes,
|
||||||
tokenProvider,
|
tokenProvider,
|
||||||
allowHosts,
|
allowHosts,
|
||||||
|
authAllowHosts: params.authAllowHosts,
|
||||||
preserveFilenames,
|
preserveFilenames,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -85,6 +87,7 @@ export async function resolveMSTeamsInboundMedia(params: {
|
|||||||
tokenProvider,
|
tokenProvider,
|
||||||
maxBytes,
|
maxBytes,
|
||||||
allowHosts,
|
allowHosts,
|
||||||
|
authAllowHosts: params.authAllowHosts,
|
||||||
preserveFilenames,
|
preserveFilenames,
|
||||||
});
|
});
|
||||||
attempts.push({
|
attempts.push({
|
||||||
|
|||||||
@@ -403,6 +403,7 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
|
|||||||
maxBytes: mediaMaxBytes,
|
maxBytes: mediaMaxBytes,
|
||||||
tokenProvider,
|
tokenProvider,
|
||||||
allowHosts: msteamsCfg?.mediaAllowHosts,
|
allowHosts: msteamsCfg?.mediaAllowHosts,
|
||||||
|
authAllowHosts: msteamsCfg?.mediaAuthAllowHosts,
|
||||||
conversationType,
|
conversationType,
|
||||||
conversationId,
|
conversationId,
|
||||||
conversationMessageId: conversationMessageId ?? undefined,
|
conversationMessageId: conversationMessageId ?? undefined,
|
||||||
|
|||||||
@@ -83,6 +83,11 @@ export type MSTeamsConfig = {
|
|||||||
* Use ["*"] to allow any host (not recommended).
|
* Use ["*"] to allow any host (not recommended).
|
||||||
*/
|
*/
|
||||||
mediaAllowHosts?: Array<string>;
|
mediaAllowHosts?: Array<string>;
|
||||||
|
/**
|
||||||
|
* Allowed host suffixes for attaching Authorization headers to inbound media retries.
|
||||||
|
* Use specific hosts only; avoid multi-tenant suffixes.
|
||||||
|
*/
|
||||||
|
mediaAuthAllowHosts?: Array<string>;
|
||||||
/** Default: require @mention to respond in channels/groups. */
|
/** Default: require @mention to respond in channels/groups. */
|
||||||
requireMention?: boolean;
|
requireMention?: boolean;
|
||||||
/** Max group/channel messages to keep as history context (0 disables). */
|
/** Max group/channel messages to keep as history context (0 disables). */
|
||||||
|
|||||||
@@ -800,6 +800,7 @@ export const MSTeamsConfigSchema = z
|
|||||||
chunkMode: z.enum(["length", "newline"]).optional(),
|
chunkMode: z.enum(["length", "newline"]).optional(),
|
||||||
blockStreamingCoalesce: BlockStreamingCoalesceSchema.optional(),
|
blockStreamingCoalesce: BlockStreamingCoalesceSchema.optional(),
|
||||||
mediaAllowHosts: z.array(z.string()).optional(),
|
mediaAllowHosts: z.array(z.string()).optional(),
|
||||||
|
mediaAuthAllowHosts: z.array(z.string()).optional(),
|
||||||
requireMention: z.boolean().optional(),
|
requireMention: z.boolean().optional(),
|
||||||
historyLimit: z.number().int().min(0).optional(),
|
historyLimit: z.number().int().min(0).optional(),
|
||||||
dmHistoryLimit: z.number().int().min(0).optional(),
|
dmHistoryLimit: z.number().int().min(0).optional(),
|
||||||
|
|||||||
Reference in New Issue
Block a user