diff --git a/.changeset/smart-rats-press.md b/.changeset/smart-rats-press.md new file mode 100644 index 000000000..ccdb07a49 --- /dev/null +++ b/.changeset/smart-rats-press.md @@ -0,0 +1,5 @@ +--- +"@chat-adapter/gchat": patch +--- + +download attachments through the Google Chat media API to prevent credential exposure diff --git a/packages/adapter-gchat/src/index.test.ts b/packages/adapter-gchat/src/index.test.ts index 924e119ac..77b50601f 100644 --- a/packages/adapter-gchat/src/index.test.ts +++ b/packages/adapter-gchat/src/index.test.ts @@ -541,7 +541,7 @@ describe("GoogleChatAdapter", () => { ); }); - it("should fall back to downloadUri fetch when media.download fails", async () => { + it("should not fetch downloadUri when media.download fails", async () => { const { adapter } = await createInitializedAdapter(); const mockDownload = vi .fn() @@ -551,13 +551,7 @@ describe("GoogleChatAdapter", () => { (adapter as any).chatApi = { media: { download: mockDownload }, }; - (adapter as any).authClient = { - getAccessToken: vi.fn().mockResolvedValue({ token: "test-token" }), - }; - const mockFetch = vi.fn().mockResolvedValue({ - ok: true, - arrayBuffer: () => Promise.resolve(new ArrayBuffer(4)), - }); + const mockFetch = vi.fn(); vi.stubGlobal("fetch", mockFetch); try { @@ -576,14 +570,11 @@ describe("GoogleChatAdapter", () => { }); const msg = adapter.parseMessage(event); - const data = await msg.attachments[0].fetchData?.(); - - expect(data).toBeInstanceOf(Buffer); - expect(mockDownload).toHaveBeenCalledTimes(1); - expect(mockFetch).toHaveBeenCalledWith( - "https://example.com/photo.png", - { headers: { Authorization: "Bearer test-token" } } + await expect(msg.attachments[0].fetchData?.()).rejects.toThrow( + "resource expired" ); + expect(mockDownload).toHaveBeenCalledTimes(1); + expect(mockFetch).not.toHaveBeenCalled(); } finally { vi.unstubAllGlobals(); } @@ -619,7 +610,7 @@ describe("GoogleChatAdapter", () => { ); }); - it("should fall back to direct URL fetch when no attachmentDataRef", async () => { + it("should not provide fetchData when only downloadUri is present", async () => { const { adapter } = await createInitializedAdapter(); const event = makeMessageEvent({ attachment: [ @@ -633,11 +624,21 @@ describe("GoogleChatAdapter", () => { }); const msg = adapter.parseMessage(event); - expect(msg.attachments[0].fetchData).toBeDefined(); - // fetchData is present because downloadUri exists + expect(msg.attachments[0].fetchData).toBeUndefined(); expect(msg.attachments[0].url).toBe("https://example.com/photo.png"); }); + it("should not rehydrate fetchData from a download URL", async () => { + const { adapter } = await createInitializedAdapter(); + const attachment = adapter.rehydrateAttachment({ + type: "file", + url: "https://example.com/document.pdf", + fetchMetadata: { url: "https://example.com/document.pdf" }, + }); + + expect(attachment.fetchData).toBeUndefined(); + }); + it("should not provide fetchData when neither resourceName nor downloadUri exist", async () => { const { adapter } = await createInitializedAdapter(); const event = makeMessageEvent({ diff --git a/packages/adapter-gchat/src/index.ts b/packages/adapter-gchat/src/index.ts index b216cacc5..3045e46d8 100644 --- a/packages/adapter-gchat/src/index.ts +++ b/packages/adapter-gchat/src/index.ts @@ -1,6 +1,5 @@ import { AdapterRateLimitError, - AuthenticationError, extractCard, extractFiles, NetworkError, @@ -1707,83 +1706,32 @@ export class GoogleChatAdapter implements Adapter { name: att.contentName || undefined, mimeType: att.contentType || undefined, fetchMetadata: Object.keys(fetchMeta).length > 0 ? fetchMeta : undefined, - fetchData: - resourceName || url - ? () => this.fetchAttachmentData(resourceName, url) - : undefined, + fetchData: resourceName + ? () => this.fetchAttachmentData(resourceName) + : undefined, }; } - protected async fetchAttachmentData( - resourceName?: string, - url?: string - ): Promise { - // Prefer media.download API (correct method for chat apps) - if (resourceName) { - try { - // `alt=media` (a StandardParameters field, and the documented - // download form for the Chat API) selects the file bytes. Without - // it the endpoint returns resource metadata instead, and the - // arraybuffer request fails with a bare 400. - const res = await this.chatApi.media.download( - { resourceName, alt: "media" }, - { responseType: "arraybuffer" } - ); - return Buffer.from(res.data as ArrayBuffer); - } catch (error) { - if (!url) { - this.handleGoogleChatError(error, "fetchAttachmentData"); - } - this.logger.warn( - "GChat media.download failed, falling back to downloadUri fetch", - { resourceName, error } - ); - } - } - - // Direct URL fetch (downloadUri) — used when no resourceName is - // available, or as a fallback when media.download fails - if (!url) { - throw new NetworkError("gchat", "No URL or resourceName available"); - } - - const auth = this.authClient; - if (typeof auth === "string" || !auth) { - throw new AuthenticationError( - "gchat", - "Cannot fetch file: no auth client configured" - ); - } - const tokenResult = await auth.getAccessToken(); - const token = - typeof tokenResult === "string" ? tokenResult : tokenResult?.token; - if (!token) { - throw new AuthenticationError("gchat", "Failed to get access token"); - } - const response = await fetch(url, { - headers: { - Authorization: `Bearer ${token}`, - }, - }); - if (!response.ok) { - throw new NetworkError( - "gchat", - `Failed to fetch file: ${response.status} ${response.statusText}` + protected async fetchAttachmentData(resourceName: string): Promise { + try { + const res = await this.chatApi.media.download( + { resourceName, alt: "media" }, + { responseType: "arraybuffer" } ); + return Buffer.from(res.data as ArrayBuffer); + } catch (error) { + this.handleGoogleChatError(error, "fetchAttachmentData"); } - const arrayBuffer = await response.arrayBuffer(); - return Buffer.from(arrayBuffer); } rehydrateAttachment(attachment: Attachment): Attachment { const resourceName = attachment.fetchMetadata?.resourceName; - const url = attachment.fetchMetadata?.url ?? attachment.url; - if (!(resourceName || url)) { + if (!resourceName) { return attachment; } return { ...attachment, - fetchData: () => this.fetchAttachmentData(resourceName, url), + fetchData: () => this.fetchAttachmentData(resourceName), }; }