From dd19b4ff79b6d2c7acd15aa591481a90eb674a3f Mon Sep 17 00:00:00 2001 From: Joel Date: Fri, 24 Jul 2026 15:37:27 +0800 Subject: [PATCH] fix: download skill files through authenticated API requests (#39499) --- .../skills/__tests__/index.spec.tsx | 155 +++++++++++++- .../orchestrate/skills/detail-dialog.tsx | 54 ++++- .../orchestrate/skills/use-skill-detail.ts | 202 ++++++++++++------ .../working-directory-panel.spec.tsx | 4 + 4 files changed, 329 insertions(+), 86 deletions(-) diff --git a/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/__tests__/index.spec.tsx b/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/__tests__/index.spec.tsx index 3edd69cf496..6907712ca02 100644 --- a/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/__tests__/index.spec.tsx +++ b/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/__tests__/index.spec.tsx @@ -5,7 +5,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { useAtomValue } from 'jotai' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { formStateToAgentSoulConfig } from '@/features/agent-v2/agent-composer/conversions' import { defaultAgentSoulConfigFormState } from '@/features/agent-v2/agent-composer/form-state' import { AgentComposerProvider } from '@/features/agent-v2/agent-composer/provider' @@ -61,6 +61,7 @@ const mocks = vi.hoisted(() => ({ downloadQueryOptions: vi.fn((_options: ConfigSkillFileQueryOptionsInput) => ({})), downloadBlob: vi.fn(), downloadUrl: vi.fn(), + fetch: vi.fn(), })) vi.mock('@langgenius/dify-ui/toast', () => ({ @@ -75,6 +76,11 @@ vi.mock('@/utils/download', () => ({ downloadUrl: mocks.downloadUrl, })) +vi.mock('@/config', async (importOriginal) => ({ + ...(await importOriginal()), + API_PREFIX: 'http://localhost:5001/console/api', +})) + vi.mock('@/service/client', () => ({ consoleQuery: { agent: { @@ -212,6 +218,12 @@ function renderAgentSkills({ describe('AgentSkills', () => { beforeEach(() => { vi.clearAllMocks() + vi.stubGlobal('fetch', mocks.fetch) + mocks.fetch.mockResolvedValue( + new Response('downloaded skill file', { + headers: { 'Content-Type': 'application/octet-stream' }, + }), + ) mocks.inspectQueryOptions.mockImplementation(({ input }) => ({ queryKey: ['inspect-skill', input], queryFn: async () => ({ @@ -234,6 +246,20 @@ describe('AgentSkills', () => { previewable: true, downloadable: true, }, + { + path: 'assets/icon.png', + name: 'icon.png', + type: 'file', + previewable: true, + downloadable: true, + }, + { + path: 'models/model.bin', + name: 'model.bin', + type: 'file', + previewable: false, + downloadable: true, + }, ], skill_md: { path: 'SKILL.md', @@ -249,15 +275,15 @@ describe('AgentSkills', () => { queryKey: ['preview-skill-file', input], queryFn: async () => ({ path: input.query.path, - binary: false, + binary: input.query.path.endsWith('.bin'), truncated: false, - text: `Preview for ${input.query.path}`, + text: input.query.path.endsWith('.bin') ? null : `Preview for ${input.query.path}`, }), })) mocks.downloadQueryOptions.mockImplementation(({ input }) => ({ queryKey: ['download-skill-file', input], queryFn: async () => ({ - url: `https://example.com/${input.query.path}`, + url: `/console/api/agent/agent-1/config/skills/Tender%20Analyzer/files/content?path=${encodeURIComponent(input.query.path)}`, }), })) mocks.skillDownloadQueryOptions.mockImplementation(({ input }) => ({ @@ -268,6 +294,10 @@ describe('AgentSkills', () => { })) }) + afterEach(() => { + vi.unstubAllGlobals() + }) + it('should prevent missing skills from being previewed or downloaded', async () => { const user = userEvent.setup() renderAgentSkills({ @@ -532,6 +562,37 @@ describe('AgentSkills', () => { }), ) }) + + await user.click(await screen.findByText('references')) + await user.click(screen.getByText('guide.md').closest('button')!) + await user.click( + screen.getByRole('button', { + name: /common\.operation\.download.*guide\.md/, + }), + ) + + await waitFor(() => { + expect(mocks.downloadQueryOptions).toHaveBeenCalledWith( + expect.objectContaining({ + input: expect.objectContaining({ + params: { + app_id: 'app-1', + name: 'Tender Analyzer', + }, + query: { + draft_type: 'draft', + node_id: 'node-1', + path: 'references/guide.md', + version_id: 'draft-1', + }, + }), + }), + ) + }) + expect(mocks.downloadBlob).toHaveBeenCalledWith({ + data: expect.any(Blob), + fileName: 'guide.md', + }) }) it('should download a whole skill package from the row action', async () => { @@ -687,10 +748,92 @@ describe('AgentSkills', () => { }), ) }) - expect(mocks.downloadUrl).toHaveBeenCalledWith({ - url: 'https://example.com/references/guide.md', + expect(mocks.fetch).toHaveBeenCalledWith( + 'http://localhost:5001/console/api/agent/agent-1/config/skills/Tender%20Analyzer/files/content?path=references%2Fguide.md', + { + credentials: 'include', + }, + ) + expect(mocks.downloadBlob).toHaveBeenCalledWith({ + data: expect.any(Blob), fileName: 'guide.md', }) + const blob = mocks.downloadBlob.mock.calls[0]?.[0].data as Blob + await expect(blob.text()).resolves.toBe('downloaded skill file') + expect(mocks.downloadUrl).not.toHaveBeenCalled() + }) + + it('should show an error when the authenticated skill member request fails', async () => { + const user = userEvent.setup() + mocks.fetch.mockResolvedValueOnce(new Response(null, { status: 401 })) + renderAgentSkills() + + await user.click(screen.getByText('Tender Analyzer').closest('button')!) + await user.click(await screen.findByText('references')) + await user.click(screen.getByText('guide.md').closest('button')!) + await user.click( + screen.getByRole('button', { + name: /common\.operation\.download.*guide\.md/, + }), + ) + + await waitFor(() => { + expect(toast.error).toHaveBeenCalledWith('common.operation.downloadFailed') + }) + expect(mocks.downloadBlob).not.toHaveBeenCalled() + }) + + it('should download binary skill members without exposing the protected URL', async () => { + const user = userEvent.setup() + renderAgentSkills() + + await user.click(screen.getByText('Tender Analyzer').closest('button')!) + await user.click(await screen.findByText('models')) + await user.click(screen.getByText('model.bin').closest('button')!) + + const downloadLink = await screen.findByRole('link', { + name: 'common.operation.download', + }) + expect(downloadLink).toHaveAttribute('href', '#') + + await user.click(downloadLink) + + await waitFor(() => { + expect(mocks.downloadBlob).toHaveBeenCalledWith({ + data: expect.any(Blob), + fileName: 'model.bin', + }) + }) + }) + + it('should preview and download image skill members from an authenticated Blob', async () => { + const user = userEvent.setup() + const createObjectURL = vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:skill-image') + const revokeObjectURL = vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => {}) + const view = renderAgentSkills() + + await user.click(screen.getByText('Tender Analyzer').closest('button')!) + await user.click(await screen.findByText('assets')) + await user.click(screen.getByText('icon.png').closest('button')!) + + const image = await screen.findByRole('img', { name: 'icon.png' }) + expect(image).toHaveAttribute('src', 'blob:skill-image') + expect(createObjectURL).toHaveBeenCalledWith(expect.any(Blob)) + + await user.click( + screen.getByRole('button', { + name: /common\.operation\.download.*icon\.png/, + }), + ) + await waitFor(() => { + expect(mocks.downloadBlob).toHaveBeenCalledWith({ + data: expect.any(Blob), + fileName: 'icon.png', + }) + }) + + view.unmount() + expect(revokeObjectURL).toHaveBeenCalledWith('blob:skill-image') }) it('should download inspected SKILL.md content as markdown', async () => { diff --git a/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/detail-dialog.tsx b/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/detail-dialog.tsx index 0806132464f..0b1c0f49525 100644 --- a/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/detail-dialog.tsx +++ b/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/detail-dialog.tsx @@ -11,6 +11,7 @@ import { } from '@langgenius/dify-ui/dialog' import { FileTreeFile } from '@langgenius/dify-ui/file-tree' import { ScrollArea } from '@langgenius/dify-ui/scroll-area' +import { useCallback } from 'react' import { useTranslation } from 'react-i18next' import Loading from '@/app/components/base/loading' import { AgentFileTree } from '../files/tree' @@ -43,6 +44,7 @@ export type AgentSkillDetail = { downloadActionLoadingTarget?: AgentSkillDetailDownloadAction | null downloadUrl?: string fileName?: string + imageData?: Blob isDownloadError?: boolean isDownloadLoading?: boolean isError?: boolean @@ -165,12 +167,39 @@ function AgentSkillDetailSectionBlock({ section }: { section: AgentSkillDetailSe ) } +function AgentSkillImagePreview({ fileName, imageData }: { fileName?: string; imageData: Blob }) { + const setImageRef = useCallback( + (image: HTMLImageElement | null) => { + if (!image) return + + const objectUrl = URL.createObjectURL(imageData) + image.src = objectUrl + + return () => { + URL.revokeObjectURL(objectUrl) + } + }, + [imageData], + ) + + return ( +
+ {fileName +
+ ) +} + function AgentFilePreviewContent({ binary, content, downloadActionLoadingTarget, downloadUrl, fileName, + imageData, isDownloadError, isDownloadLoading, isError, @@ -183,6 +212,7 @@ function AgentFilePreviewContent({ downloadActionLoadingTarget?: AgentSkillDetailDownloadAction | null downloadUrl?: string fileName?: string + imageData?: Blob isDownloadError?: boolean isDownloadLoading?: boolean isError?: boolean @@ -210,16 +240,19 @@ function AgentFilePreviewContent({ ) } - if (isImage && downloadUrl) { - return ( -
- {fileName -
- ) + if (isImage) { + if (imageData) return + if (downloadUrl) { + return ( +
+ {fileName +
+ ) + } } if (binary) { @@ -390,6 +423,7 @@ export function AgentSkillDetailDialog({ downloadActionLoadingTarget={detail.filePreview.downloadActionLoadingTarget} downloadUrl={detail.filePreview.downloadUrl} fileName={detail.filePreview.fileName} + imageData={detail.filePreview.imageData} isDownloadError={detail.filePreview.isDownloadError} isDownloadLoading={detail.filePreview.isDownloadLoading} isError={detail.filePreview.isError} diff --git a/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/use-skill-detail.ts b/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/use-skill-detail.ts index 68995149a14..459375f85bd 100644 --- a/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/use-skill-detail.ts +++ b/web/features/agent-v2/agent-detail/configure/components/orchestrate/skills/use-skill-detail.ts @@ -2,14 +2,44 @@ import type { AgentConfigSkillFileResponse } from '@dify/contracts/api/console/agent/types.gen' import type { AgentConfigApiContext } from '../config-context' -import type { AgentSkillDetail } from './detail-dialog' +import type { AgentSkillDetail, AgentSkillDetailDownloadAction } from './detail-dialog' import type { AgentFileNode, AgentSkill } from '@/features/agent-v2/agent-composer/form-state' -import { useQuery, useQueryClient } from '@tanstack/react-query' +import { toast } from '@langgenius/dify-ui/toast' +import { queryOptions, skipToken, useQuery, useQueryClient } from '@tanstack/react-query' import { useCallback, useMemo, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { API_PREFIX } from '@/config' import { consoleQuery } from '@/service/client' -import { downloadBlob, downloadUrl } from '@/utils/download' +import { downloadBlob } from '@/utils/download' import { getDriveFileIconType } from '../files/file-icon' +const skillFileContentQueryKey = (url?: string) => ['agent-v2', 'skill-file-content', url] as const + +const resolveSkillFileContentUrl = (url: string) => { + const consoleApiUrl = new URL(API_PREFIX, globalThis.location.origin) + const consoleApiBaseUrl = consoleApiUrl.href.endsWith('/') + ? consoleApiUrl + : new URL(`${consoleApiUrl.href}/`) + + return new URL(url, consoleApiBaseUrl).toString() +} + +const fetchSkillFileContent = async (url: string) => { + const response = await globalThis.fetch(resolveSkillFileContentUrl(url), { + credentials: 'include', + }) + if (!response.ok) throw new Error(`Failed to download skill file: ${response.status}`) + + return response.blob() +} + +const getSkillFileContentQueryOptions = (url: string) => + queryOptions({ + queryKey: skillFileContentQueryKey(url), + queryFn: () => fetchSkillFileContent(url), + staleTime: Infinity, + }) + const isSkillFolder = (file: AgentConfigSkillFileResponse) => file.type === 'directory' const toSkillFileNode = (item: AgentConfigSkillFileResponse): AgentFileNode => { @@ -22,7 +52,7 @@ const toSkillFileNode = (item: AgentConfigSkillFileResponse): AgentFileNode => { icon: isSkillFolder(item) ? 'folder' : getDriveFileIconType({ - fileKind: item.type, + fileKind: undefined, fileName, mimeType: undefined, }), @@ -144,8 +174,11 @@ export function useAgentSkillDetail({ isOpen: boolean skill: AgentSkill }): AgentSkillDetail { + const { t: tCommon } = useTranslation('common') const queryClient = useQueryClient() const [selectedFileId, setSelectedFileId] = useState() + const [downloadActionLoadingTarget, setDownloadActionLoadingTarget] = + useState(null) const agentSkillInspectQuery = useQuery({ ...consoleQuery.agent.byAgentId.config.skills.byName.inspect.get.queryOptions({ input: { @@ -229,8 +262,7 @@ export function useAgentSkillDetail({ }) const previewQuery = apiContext.workflow ? workflowPreviewQuery : agentPreviewQuery const isImagePreviewFile = selectedFile?.icon === 'image' - const shouldDownloadPreviewFile = - isOpen && !!selectedPreviewPath && (isImagePreviewFile || !!previewQuery.data?.binary) + const shouldLoadImagePreview = isOpen && !!selectedPreviewPath && isImagePreviewFile const agentDownloadQuery = useQuery({ ...consoleQuery.agent.byAgentId.config.skills.byName.files.download.get.queryOptions({ input: { @@ -245,7 +277,7 @@ export function useAgentSkillDetail({ }, }, }), - enabled: shouldDownloadPreviewFile && !apiContext.workflow, + enabled: shouldLoadImagePreview && !apiContext.workflow, }) const workflowDownloadQuery = useQuery({ ...consoleQuery.apps.byAppId.agent.config.skills.byName.files.download.get.queryOptions({ @@ -262,69 +294,96 @@ export function useAgentSkillDetail({ }, }, }), - enabled: shouldDownloadPreviewFile && !!apiContext.workflow, + enabled: shouldLoadImagePreview && !!apiContext.workflow, }) const downloadQuery = apiContext.workflow ? workflowDownloadQuery : agentDownloadQuery - const handleDownloadFile = useCallback(async () => { - if (!selectedFile) return - - const file = selectedFile - const path = file.configName ?? file.id - const isSkillMdFile = path === inspectQuery.data?.skill_md.path || file.name === 'SKILL.md' - - if (isSkillMdFile && inspectQuery.data?.skill_md.text !== undefined) { - downloadBlob({ - data: new Blob([inspectQuery.data.skill_md.text], { type: 'text/markdown;charset=utf-8' }), - fileName: file.name, - }) - return - } - - if (apiContext.workflow) { - const result = await queryClient.fetchQuery( - consoleQuery.apps.byAppId.agent.config.skills.byName.files.download.get.queryOptions({ - input: { - params: { - app_id: apiContext.workflow.appId, - name: skill.name, - }, - query: { - node_id: apiContext.workflow.nodeId, - path, - draft_type: apiContext.draftType, - version_id: apiContext.versionId, - }, - }, - }), - ) - downloadUrl({ url: result.url, fileName: file.name }) - return - } - - const result = await queryClient.fetchQuery( - consoleQuery.agent.byAgentId.config.skills.byName.files.download.get.queryOptions({ - input: { - params: { - agent_id: apiContext.agentId, - name: skill.name, - }, - query: { - path, - draft_type: apiContext.draftType, - version_id: apiContext.versionId, - }, + const skillFileContentUrl = downloadQuery.data?.url + const imageContentQuery = useQuery( + skillFileContentUrl + ? { + ...getSkillFileContentQueryOptions(skillFileContentUrl), + enabled: shouldLoadImagePreview, + } + : { + queryKey: skillFileContentQueryKey(), + queryFn: skipToken, }, - }), - ) - downloadUrl({ url: result.url, fileName: file.name }) - }, [ - apiContext, - inspectQuery.data?.skill_md.path, - inspectQuery.data?.skill_md.text, - queryClient, - selectedFile, - skill.name, - ]) + ) + const handleDownloadFile = useCallback( + async (action: AgentSkillDetailDownloadAction) => { + if (!selectedFile) return + + const file = selectedFile + const path = file.configName ?? file.id + const isSkillMdFile = path === inspectQuery.data?.skill_md.path || file.name === 'SKILL.md' + + if (isSkillMdFile && inspectQuery.data?.skill_md.text !== undefined) { + downloadBlob({ + data: new Blob([inspectQuery.data.skill_md.text], { + type: 'text/markdown;charset=utf-8', + }), + fileName: file.name, + }) + return + } + + setDownloadActionLoadingTarget(action) + try { + if (apiContext.workflow) { + const result = await queryClient.fetchQuery( + consoleQuery.apps.byAppId.agent.config.skills.byName.files.download.get.queryOptions({ + input: { + params: { + app_id: apiContext.workflow.appId, + name: skill.name, + }, + query: { + node_id: apiContext.workflow.nodeId, + path, + draft_type: apiContext.draftType, + version_id: apiContext.versionId, + }, + }, + }), + ) + const data = await queryClient.fetchQuery(getSkillFileContentQueryOptions(result.url)) + downloadBlob({ data, fileName: file.name }) + return + } + + const result = await queryClient.fetchQuery( + consoleQuery.agent.byAgentId.config.skills.byName.files.download.get.queryOptions({ + input: { + params: { + agent_id: apiContext.agentId, + name: skill.name, + }, + query: { + path, + draft_type: apiContext.draftType, + version_id: apiContext.versionId, + }, + }, + }), + ) + const data = await queryClient.fetchQuery(getSkillFileContentQueryOptions(result.url)) + downloadBlob({ data, fileName: file.name }) + } catch { + toast.error(tCommon(($) => $['operation.downloadFailed'])) + } finally { + setDownloadActionLoadingTarget(null) + } + }, + [ + apiContext, + inspectQuery.data?.skill_md.path, + inspectQuery.data?.skill_md.text, + queryClient, + selectedFile, + skill.name, + tCommon, + ], + ) return { description, @@ -335,10 +394,13 @@ export function useAgentSkillDetail({ content: isSkillMdSelected ? (inspectQuery.data?.skill_md.text ?? undefined) : (previewQuery.data?.text ?? undefined), - downloadUrl: downloadQuery.data?.url, + downloadActionLoadingTarget, fileName: selectedFile?.name, - isDownloadError: downloadQuery.isError, - isDownloadLoading: shouldDownloadPreviewFile && downloadQuery.isPending, + imageData: imageContentQuery.data, + isDownloadError: + shouldLoadImagePreview && (downloadQuery.isError || imageContentQuery.isError), + isDownloadLoading: + shouldLoadImagePreview && (downloadQuery.isPending || imageContentQuery.isPending), isError: isSkillMdSelected ? inspectQuery.isError : !!selectedPreviewPath && previewQuery.isError, diff --git a/web/features/agent-v2/agent-detail/configure/components/preview/__tests__/working-directory-panel.spec.tsx b/web/features/agent-v2/agent-detail/configure/components/preview/__tests__/working-directory-panel.spec.tsx index b9c40fbceea..bda6f3ae5b5 100644 --- a/web/features/agent-v2/agent-detail/configure/components/preview/__tests__/working-directory-panel.spec.tsx +++ b/web/features/agent-v2/agent-detail/configure/components/preview/__tests__/working-directory-panel.spec.tsx @@ -283,6 +283,8 @@ describe('AgentWorkingDirectoryPanel', () => { it('should preview sandbox images with the uploaded file url', async () => { const user = userEvent.setup() + const upload = createDeferred<{ url: string }>() + mocks.sandboxFileUploadClientPost.mockReturnValueOnce(upload.promise) renderWorkingDirectoryPanel() await user.click(await screen.findByText('chart.png')) @@ -290,6 +292,8 @@ describe('AgentWorkingDirectoryPanel', () => { await waitFor(() => { expect(mocks.sandboxFileUploadClientPost).toHaveBeenCalled() }) + upload.resolve({ url: 'https://example.com/chart.png' }) + const image = await screen.findByAltText('chart.png') expect(image).toHaveAttribute('src', 'https://example.com/chart.png') expect(mocks.sandboxFileUploadClientPost).toHaveBeenCalledWith({