diff --git a/extensions/msteams/src/attachments.test.ts b/extensions/msteams/src/attachments.test.ts index 11aa475abf71..a8e4d9b22d3e 100644 --- a/extensions/msteams/src/attachments.test.ts +++ b/extensions/msteams/src/attachments.test.ts @@ -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(); }); }); diff --git a/extensions/msteams/src/attachments/shared.test.ts b/extensions/msteams/src/attachments/shared.test.ts index 8c66f6f7a572..4f77e8ed29d6 100644 --- a/extensions/msteams/src/attachments/shared.test.ts +++ b/extensions/msteams/src/attachments/shared.test.ts @@ -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); diff --git a/extensions/msteams/src/attachments/shared.ts b/extensions/msteams/src/attachments/shared.ts index 914425391eaa..0db0aadc6426 100644 --- a/extensions/msteams/src/attachments/shared.ts +++ b/extensions/msteams/src/attachments/shared.ts @@ -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}`), + ); } /**