-
Notifications
You must be signed in to change notification settings - Fork 60
feat: add modular artifact format validation #2158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
8e88cc2
feat: add modular artifact format validation
qinxuye 19fbc07
fix: preserve artifact validator failure diagnostics
qinxuye 9ff1357
chore: remove artifact validation documentation
qinxuye 76bd114
fix: address artifact validation review feedback
qinxuye 63008c4
fix: avoid unsupported artifact validation verdicts
qinxuye 6c3b8b6
fix: type resource limits as a fixed pair
qinxuye 02923e8
fix: enforce artifact validation trust boundaries
qinxuye 2e0c1cc
fix: clarify artifact validation limits and diagnostics
qinxuye File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
116 changes: 116 additions & 0 deletions
116
frontend/src/components/file/artifact-validation.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| /// <reference types="@testing-library/jest-dom/vitest" /> | ||
| import React from 'react' | ||
| import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' | ||
| import { afterEach, describe, expect, it, vi } from 'vitest' | ||
| import { ArtifactValidation } from './artifact-validation' | ||
| import { FileAccessProvider, defaultFileAccessPolicy, createPublicFileAccessPolicy } from '@/contexts/file-access-context' | ||
| import { I18nProvider } from '@/contexts/i18n-context' | ||
| import { InlineFilePreview } from './inline-file-preview' | ||
|
|
||
| const reportHeaders = { 'content-type': 'application/vnd.xagent.validation+json' } | ||
| const report = (status: string, message = '', supported = true) => new Response(JSON.stringify({ status, supported, sha256: '0'.repeat(64), checks: [{ status, message }] }), { headers: reportHeaders }) | ||
| const wrap = (children: React.ReactNode, request = vi.fn(), extra = {}) => <I18nProvider> | ||
| <FileAccessProvider policy={{ ...defaultFileAccessPolicy, request, ...extra }}>{children}</FileAccessProvider> | ||
| </I18nProvider> | ||
|
|
||
| afterEach(() => { cleanup(); vi.unstubAllGlobals(); vi.useRealTimers(); localStorage.removeItem('app_locale') }) | ||
|
|
||
| describe('ArtifactValidation', () => { | ||
| it.each(['valid', 'invalid', 'unchecked'] as const)('shows %s without hiding repair/download access', async status => { | ||
| const request = vi.fn().mockResolvedValue(report(status, status === 'invalid' ? 'Corrupt package' : '')) | ||
| const { container } = render(wrap(<ArtifactValidation fileId="one"><a href="/download">artifact</a></ArtifactValidation>, request)) | ||
| expect(container.querySelector('[data-artifact-validation="checking"]')).toBeTruthy() | ||
| await waitFor(() => expect(container.querySelector(`[data-artifact-validation="${status}"]`)).toBeTruthy()) | ||
| expect(screen.getByRole('link', { name: 'artifact' })).toHaveAttribute('href', '/download') | ||
| expect(request).toHaveBeenCalledWith('/api/files/preview/one?validation_only=true', expect.objectContaining({ cache: 'no-store', signal: expect.any(AbortSignal) })) | ||
| }) | ||
|
|
||
| it('rechecks the same file id after repair without reusing a cached pass', async () => { | ||
| const request = vi.fn().mockResolvedValueOnce(report('valid')).mockResolvedValueOnce(report('invalid', 'broken')) | ||
| const { container } = render(wrap(<ArtifactValidation fileId="one">file</ArtifactValidation>, request)) | ||
| await screen.findByText('Format readable · content not verified') | ||
| fireEvent.click(screen.getByRole('button', { name: 'Recheck' })) | ||
| expect(container.querySelector('[data-artifact-validation="checking"]')).toBeTruthy() | ||
| await screen.findByText('File validation failed · repair required') | ||
| expect(request).toHaveBeenCalledTimes(2) | ||
| }) | ||
|
|
||
| it.each([ | ||
| new Response('', { status: 403 }), new Response('', { status: 500 }), report('made-up'), new Response('not json'), | ||
| new Response('{"status":"valid"}', { headers: reportHeaders }), | ||
| new Response('{"status":"valid","checks":[{"status":"valid"}]}', { headers: reportHeaders }), | ||
| new Response(JSON.stringify({ status: 'valid', sha256: '0'.repeat(64), checks: [{ status: 'unchecked' }] }), { headers: reportHeaders }), | ||
| new Response(JSON.stringify({ status: 'valid', sha256: '0'.repeat(64), checks: [{ status: 'valid' }] }), { headers: { 'content-type': 'application/json' } }), | ||
| ])('does not treat a failed/malformed request as a pass', async response => { | ||
| render(wrap(<ArtifactValidation fileId="one">file</ArtifactValidation>, vi.fn().mockResolvedValue(response))) | ||
| await screen.findByText('Unable to request file validation · try again') | ||
| expect(screen.queryByText('File not checked')).toBeNull() | ||
| }) | ||
|
|
||
| it('ignores an old response after the file changes', async () => { | ||
| let finish!: (response: Response) => void | ||
| const request = vi.fn().mockImplementationOnce(() => new Promise(resolve => { finish = resolve })).mockResolvedValueOnce(report('invalid')) | ||
| const policy = { ...defaultFileAccessPolicy, request } | ||
| const view = (fileId: string) => <I18nProvider><FileAccessProvider policy={policy}><ArtifactValidation fileId={fileId}>file</ArtifactValidation></FileAccessProvider></I18nProvider> | ||
| const { rerender } = render(view('one')) | ||
| rerender(view('two')) | ||
| await screen.findByText('File validation failed · repair required') | ||
| await act(async () => { finish(report('valid')) }) | ||
| expect(screen.queryByText('Format readable · content not verified')).toBeNull() | ||
| expect(request.mock.calls[0][1].signal.aborted).toBe(true) | ||
| }) | ||
|
|
||
| it('does not request validation for policies without the capability', () => { | ||
| const request = vi.fn() | ||
| render(wrap(<ArtifactValidation fileId="one">file</ArtifactValidation>, request, { validationUrl: undefined })) | ||
| expect(request).not.toHaveBeenCalled() | ||
| expect(screen.queryByRole('status')).toBeNull() | ||
| }) | ||
|
|
||
| it('uses the public scoped token and strips ambient authorization', async () => { | ||
| const fetchMock = vi.fn().mockResolvedValue(report('valid')) | ||
| vi.stubGlobal('fetch', fetchMock) | ||
| const policy = createPublicFileAccessPolicy('guest-token') | ||
| render(<I18nProvider><FileAccessProvider policy={policy}><ArtifactValidation fileId="one">file</ArtifactValidation></FileAccessProvider></I18nProvider>) | ||
| await screen.findByText('Format readable · content not verified') | ||
| expect(fetchMock.mock.calls[0][0]).toContain('token=guest-token&validation_only=true') | ||
| expect(fetchMock.mock.calls[0][1].headers.has('Authorization')).toBe(false) | ||
| }) | ||
|
|
||
| it('hides unsupported-format controls using the server capability, retaining the file', async () => { | ||
| const request = vi.fn().mockResolvedValue(report('unchecked', 'No validator is installed for this format.', false)) | ||
| render(wrap(<InlineFilePreview source={{ fileId: '123e4567-e89b-12d3-a456-426614174000', filename: 'notes.txt' }} />, request)) | ||
| await waitFor(() => expect(screen.queryByRole('status')).toBeNull()) | ||
| expect(request).toHaveBeenCalledTimes(1) | ||
| expect(screen.queryByRole('button', { name: 'Recheck' })).toBeNull() | ||
| expect(screen.getByRole('link')).toHaveTextContent('notes.txt') | ||
| }) | ||
|
|
||
| it('distinguishes a network error and permits a successful retry', async () => { | ||
| const request = vi.fn().mockRejectedValueOnce(new TypeError('offline')).mockResolvedValueOnce(report('valid')) | ||
| render(wrap(<ArtifactValidation fileId="one">file</ArtifactValidation>, request)) | ||
| await screen.findByText('Unable to request file validation · try again') | ||
| fireEvent.click(screen.getByRole('button', { name: 'Recheck' })) | ||
| await screen.findByText('Format readable · content not verified') | ||
| }) | ||
|
|
||
| it('reports a client timeout as a request error, not a completed unchecked report', async () => { | ||
| vi.useFakeTimers() | ||
| const request = vi.fn().mockImplementation((_url, options) => new Promise((_resolve, reject) => { | ||
| options.signal.addEventListener('abort', () => reject(new DOMException('Aborted', 'AbortError'))) | ||
| })) | ||
| render(wrap(<ArtifactValidation fileId="one">file</ArtifactValidation>, request)) | ||
| await act(async () => { await vi.advanceTimersByTimeAsync(20_001) }) | ||
| expect(screen.getByText('Unable to request file validation · try again')).toBeInTheDocument() | ||
| expect(request.mock.calls[0][1].signal.aborted).toBe(true) | ||
| }) | ||
|
|
||
| it('uses translated machine-status labels instead of raw English parser diagnostics', async () => { | ||
| localStorage.setItem('app_locale', 'zh') | ||
| const request = vi.fn().mockResolvedValue(report('invalid', 'PDF header is missing.')) | ||
| render(wrap(<ArtifactValidation fileId="one">file</ArtifactValidation>, request)) | ||
| await screen.findByText('文件校验失败 · 需要修复') | ||
| expect(screen.queryByText('PDF header is missing.')).toBeNull() | ||
| expect(screen.getByRole('button', { name: '重新检查' })).toBeInTheDocument() | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import React, { useEffect, useState } from 'react' | ||
| import { useFileAccess } from '@/contexts/file-access-context' | ||
| import { useI18n } from '@/contexts/i18n-context' | ||
|
|
||
| type Status = 'valid' | 'invalid' | 'unchecked' | ||
| type DisplayReport = { status: Status | 'error'; supported: boolean } | ||
| const STATUSES: ReadonlySet<string> = new Set(['valid', 'invalid', 'unchecked']) | ||
|
|
||
| async function readReport(response: Response): Promise<DisplayReport> { | ||
| if (!response.ok) throw new Error('Validation unavailable') | ||
| // An older backend may ignore validation_only and return the attachment | ||
| // itself. Never interpret arbitrary JSON file contents as a report. | ||
| if (response.headers.get('content-type')?.split(';')[0] !== 'application/vnd.xagent.validation+json') { | ||
| throw new Error('Not a validation response') | ||
| } | ||
| const data = await response.json() | ||
| if (!data || !STATUSES.has(data.status) || | ||
| !Array.isArray(data.checks) || !data.checks.length || | ||
| !data.checks.every((c: { status?: unknown } | null) => c && STATUSES.has(String(c.status)))) { | ||
| throw new Error('Invalid report') | ||
| } | ||
| if (data.status !== 'unchecked' && | ||
| (typeof data.sha256 !== 'string' || !/^[a-f0-9]{64}$/.test(data.sha256))) { | ||
| throw new Error('Missing snapshot') | ||
| } | ||
| if (data.status === 'valid' && !data.checks.every((c: { status: string }) => c.status === 'valid')) { | ||
| throw new Error('Incomplete checks') | ||
| } | ||
| // The backend owns format support. Do not duplicate a suffix allowlist in | ||
| // the UI or show an endless Recheck action for formats without a validator. | ||
| // Machine statuses drive localized presentation; English parser diagnostics | ||
| // remain in the API/model report rather than leaking into the user's locale. | ||
| return { status: data.status, supported: data.supported !== false } | ||
| } | ||
|
|
||
| /** Server-authoritative, current-byte checks, independent of preview renderers. */ | ||
| export function ArtifactValidation({ fileId, children }: { | ||
| fileId: string | ||
| children: React.ReactNode | ||
| }) { | ||
| const policy = useFileAccess() | ||
| const { t } = useI18n() | ||
| const [attempt, setAttempt] = useState(0) | ||
| const [result, setResult] = useState<DisplayReport & { key: string }>() | ||
| const url = policy.validationUrl?.(fileId) | ||
| const key = `${url}:${attempt}` | ||
|
|
||
| useEffect(() => { | ||
| if (!url) return | ||
| let active = true | ||
| const controller = new AbortController() | ||
| const timeout = setTimeout(() => controller.abort(), 20_000) | ||
| const check = async () => { | ||
| try { | ||
| const response = await policy.request(url, { signal: controller.signal, cache: 'no-store' }) | ||
| const report = await readReport(response) | ||
| if (active) setResult({ key, ...report }) | ||
| } catch { | ||
| if (active) setResult({ key, status: 'error', supported: true }) | ||
| } finally { | ||
| clearTimeout(timeout) | ||
| } | ||
| } | ||
| void check() | ||
| return () => { | ||
| active = false | ||
| clearTimeout(timeout) | ||
| controller.abort() | ||
| } | ||
| }, [key, url, policy]) | ||
|
|
||
| if (!url) return <>{children}</> | ||
| const current = result?.key === key ? result : undefined | ||
| if (current?.supported === false) return <>{children}</> | ||
| const label = current?.status ?? 'checking' | ||
| return ( | ||
| <div data-artifact-validation={label}> | ||
| <div className="flex items-center gap-2 py-1 text-xs text-muted-foreground" role="status"> | ||
| <span className={label === 'invalid' ? 'text-destructive' : undefined}> | ||
| {t(`files.validation.${label}`)} | ||
| </span> | ||
| {current ? ( | ||
| <button type="button" className="underline" onClick={() => setAttempt(n => n + 1)}> | ||
| {t('files.validation.recheck')} | ||
| </button> | ||
| ) : null} | ||
| </div> | ||
| <React.Fragment key={key}>{children}</React.Fragment> | ||
| </div> | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.