mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix(msteams): require dot boundary for shared-link host suffix match (#123046)
* fix(msteams): require dot boundary for shared-link host suffix match Look-alike hosts such as evil1drv.ms, notonedrive.com, or fakeonedrive.live.com satisfied the bare endsWith() check in isGraphSharedLinkUrl and were rewritten to the Graph shares endpoint. Match bare suffixes on a label boundary instead: exact host or a dot-prefixed suffix. * test(msteams): cover lookalike download routing * fix(msteams): require https for shared links --------- Co-authored-by: sallyom <somalley@redhat.com>
This commit is contained in:
@@ -718,9 +718,15 @@ describe("msteams attachments", () => {
|
||||
expect(tokenProvider.getAccessToken).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls through to direct fetch for non-shared-link URLs", async () => {
|
||||
const directUrl = createTestUrl("direct.pdf");
|
||||
const fetchMock = createOkFetchMock(CONTENT_TYPE_APPLICATION_PDF, "pdf");
|
||||
it("keeps look-alike hosts out of Graph shares and auth fallback", async () => {
|
||||
const directUrl = "https://notonedrive.com/direct.pdf";
|
||||
const tokenProvider = createTokenProvider();
|
||||
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
|
||||
const url = resolveRequestUrl(input);
|
||||
return url.startsWith(GRAPH_SHARES_URL_PREFIX)
|
||||
? createTextResponse("unauthorized", 401)
|
||||
: createBufferResponse(PDF_BUFFER, CONTENT_TYPE_APPLICATION_PDF);
|
||||
});
|
||||
detectMimeMock.mockResolvedValueOnce(CONTENT_TYPE_APPLICATION_PDF);
|
||||
saveMediaBufferMock.mockResolvedValueOnce({
|
||||
id: "saved.pdf",
|
||||
@@ -729,9 +735,13 @@ describe("msteams attachments", () => {
|
||||
contentType: CONTENT_TYPE_APPLICATION_PDF,
|
||||
});
|
||||
|
||||
const media = await downloadAttachmentsWithFetch(
|
||||
createPdfAttachments(directUrl),
|
||||
fetchMock,
|
||||
const media = await downloadMSTeamsAttachments(
|
||||
buildDownloadParams(createPdfAttachments(directUrl), {
|
||||
tokenProvider,
|
||||
allowHosts: ["notonedrive.com", GRAPH_HOST],
|
||||
authAllowHosts: [GRAPH_HOST],
|
||||
fetchFn: asFetchFn(fetchMock),
|
||||
}),
|
||||
);
|
||||
|
||||
expectAttachmentMediaLength(media, 1);
|
||||
@@ -742,6 +752,25 @@ describe("msteams attachments", () => {
|
||||
// Should have hit the original host, NOT graph shares.
|
||||
expect(calledUrls).toContain(directUrl);
|
||||
expect(calledUrls.some((url) => url.startsWith(GRAPH_SHARES_URL_PREFIX))).toBe(false);
|
||||
expect(tokenProvider.getAccessToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects non-HTTPS shared-link hosts before fetch or auth fallback", async () => {
|
||||
const tokenProvider = createTokenProvider();
|
||||
const fetchMock = vi.fn(async () => createTextResponse("unauthorized", 401));
|
||||
|
||||
await downloadAttachmentsWithFetch(
|
||||
createPdfAttachments("http://onedrive.com/direct.pdf"),
|
||||
fetchMock,
|
||||
{
|
||||
tokenProvider,
|
||||
allowHosts: ["onedrive.com", GRAPH_HOST],
|
||||
authAllowHosts: [GRAPH_HOST],
|
||||
},
|
||||
{ expectFetchCalled: false },
|
||||
);
|
||||
|
||||
expect(tokenProvider.getAccessToken).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -534,6 +534,11 @@ describe("Graph shared-link helpers", () => {
|
||||
["https://graph.microsoft.com/v1.0/me", false],
|
||||
["https://smba.trafficmanager.net/amer/v3", false],
|
||||
["https://example.com/file.pdf", false],
|
||||
["https://notonedrive.com/x", false],
|
||||
["https://evil1drv.ms/x", false],
|
||||
["https://fakeonedrive.live.com/x", false],
|
||||
["https://evilsharepoint.com/x", false],
|
||||
["http://onedrive.com/x", false],
|
||||
["not-a-url", false],
|
||||
])("isGraphSharedLinkUrl(%s) === %s", (url, expected) => {
|
||||
expect(isGraphSharedLinkUrl(url)).toBe(expected);
|
||||
|
||||
@@ -165,16 +165,20 @@ const GRAPH_SHARED_LINK_HOST_SUFFIXES = [
|
||||
* than directly.
|
||||
*/
|
||||
function isGraphSharedLinkUrl(url: string): boolean {
|
||||
let host: string;
|
||||
let parsed: URL;
|
||||
try {
|
||||
host = normalizeLowercaseStringOrEmpty(new URL(url).hostname);
|
||||
parsed = new URL(url);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
if (!host) {
|
||||
const host = normalizeLowercaseStringOrEmpty(parsed.hostname);
|
||||
if (parsed.protocol !== "https:" || !host) {
|
||||
return false;
|
||||
}
|
||||
return GRAPH_SHARED_LINK_HOST_SUFFIXES.some((suffix) => host === suffix || host.endsWith(suffix));
|
||||
// Only HTTPS URLs on a DNS label boundary may select the authenticated Graph path.
|
||||
return GRAPH_SHARED_LINK_HOST_SUFFIXES.some(
|
||||
(suffix) => host === suffix || host.endsWith(suffix.startsWith(".") ? suffix : `.${suffix}`),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user