Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions example.env
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,11 @@ XAGENT_EXTERNAL_SKILLS_LIBRARY_DIRS=""
# Supports raw bytes or human-readable values like 100M, 1G, 512K.
XAGENT_MAX_UPLOAD_SIZE="100M"

# Format-readability validation for generated artifacts. Unsupported formats,
# absent optional parsers and exhausted budgets are reported as unchecked.
XAGENT_ARTIFACT_VALIDATION_MAX_BYTES=33554432
XAGENT_ARTIFACT_VALIDATION_TIMEOUT_SECONDS=8

# Durable file storage for user-visible uploads and registered workspace outputs.
# Defaults to file://$XAGENT_STORAGE_ROOT/files for local development.
# For S3-compatible storage, use a URI with bucket and optional prefix.
Expand Down
85 changes: 85 additions & 0 deletions frontend/src/components/file/artifact-validation.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/// <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 = '') => new Response(JSON.stringify({ status, 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() })

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 }), 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('File not checked')
})

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('is wired to inline attachments without requiring a skill or tool trace', async () => {
const request = vi.fn().mockResolvedValue(report('unchecked'))
render(wrap(<InlineFilePreview source={{ fileId: '123e4567-e89b-12d3-a456-426614174000', filename: 'notes.txt' }} />, request))
await screen.findByText('File not checked')
expect(screen.getByRole('link')).toHaveTextContent('notes.txt')
})
})
90 changes: 90 additions & 0 deletions frontend/src/components/file/artifact-validation.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
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; message: string }

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()
const statuses = ['valid', 'invalid', 'unchecked']
Comment thread
qinxuye marked this conversation as resolved.
Outdated
if (!data || !statuses.includes(data.status) ||
!Array.isArray(data.checks) || !data.checks.length ||
!data.checks.every((c: { status?: unknown } | null) => c && statuses.includes(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')
}
const message = data.checks
.filter((c: { status: string; message?: unknown }) => c.status !== 'valid' && typeof c.message === 'string')
.map((c: { message: string }) => c.message).join(' ')
return { status: data.status, message }
}

/** 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: 'unchecked', message: '' })
} 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
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>
{current?.message ? <p className="text-xs text-muted-foreground">{current.message}</p> : null}
<React.Fragment key={key}>{children}</React.Fragment>
</div>
)
}
6 changes: 6 additions & 0 deletions frontend/src/components/file/inline-file-preview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const apiRequestMock = vi.hoisted(() => vi.fn())
const toastErrorMock = vi.hoisted(() => vi.fn())

// Validation has its own server-boundary/integration suite; these tests isolate
// the existing renderer and streaming contracts from that separate request.
vi.mock('@/components/file/artifact-validation', () => ({
ArtifactValidation: ({ children }: { children: React.ReactNode }) => <>{children}</>,
}))

vi.mock('@/components/ui/sonner', () => ({
toast: { error: toastErrorMock },
}))
Expand Down
15 changes: 14 additions & 1 deletion frontend/src/components/file/inline-file-preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { FileText, Loader2, Video, Volume2 } from 'lucide-react'
import { DocxPreviewRenderer } from '@/components/file/docx-preview-renderer'
import { ExcelPreviewRenderer } from '@/components/file/excel-preview-renderer'
import { PptxPreviewRenderer } from '@/components/file/pptx-preview-renderer'
import { ArtifactValidation } from '@/components/file/artifact-validation'
import { toast } from '@/components/ui/sonner'
import { cn, getApiUrl } from '@/lib/utils'
import { useFileAccess, type FileAccessPolicy } from '@/contexts/file-access-context'
Expand Down Expand Up @@ -865,7 +866,19 @@ function ExternalPreviewPlaceholder({
)
}

export function InlineFilePreview({
export function InlineFilePreview(props: InlineFilePreviewProps) {
const fileAccess = useFileAccess()
const fileId = props.source.fileId && resolveInlineFileId(props.source.fileId)
const kind = getInlineFilePreviewKind(props.source)
const content = <InlineFilePreviewContent {...props} />
// External URLs never enter the authenticated validation boundary. Audio and
// video keep progressive delivery; their decoders are not covered yet.
if (!fileId || !fileAccess.validationUrl || kind === 'audio' || kind === 'video' ||
!getPreviewUrlTrust({ ...props.source, fileId }, getApiUrl()).isTrusted) return content
return <ArtifactValidation key={fileId} fileId={fileId}>{content}</ArtifactValidation>
Comment thread
qinxuye marked this conversation as resolved.
}

function InlineFilePreviewContent({
source,
className,
imageClassName,
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/contexts/file-access-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface FileAccessPolicy {
inlineDownloadUrl: (fileId: string) => string
relativePreviewUrl: (fileId: string, relativePath: string) => string
pdfPreviewUrl?: (fileId: string) => string
validationUrl?: (fileId: string) => string
/**
* Execute under this policy's credential boundary. The built-in default
* attaches Bearer authorization. The public policy forces same-origin
Expand Down Expand Up @@ -77,6 +78,7 @@ export const defaultFileAccessPolicy: FileAccessPolicy = {
return getApiUrl() ? url.toString() : `${url.pathname}${url.search}${url.hash}`
},
pdfPreviewUrl: (fileId) => buildUrl(`${FILES_API_PREFIX}/preview-pdf/${encodeFileId(fileId)}`),
validationUrl: (fileId) => buildUrl(`${FILES_API_PREFIX}/preview/${encodeFileId(fileId)}?validation_only=true`),
request: apiRequest,
listFiles: (query) => {
const params = new URLSearchParams({ page: "1", size: "20" })
Expand Down Expand Up @@ -197,6 +199,7 @@ export function createPublicFileAccessPolicy(accessToken: string): FileAccessPol
downloadUrl: inlineDownloadUrl,
inlinePreviewUrl,
inlineDownloadUrl,
validationUrl: (fileId) => `${inlinePreviewUrl(fileId)}&validation_only=true`,
relativePreviewUrl: (fileId, relativePath) => {
const url = new URL(
inlinePreviewUrl(fileId),
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/i18n/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,13 @@ Build when you need.`,
page: "Page {page} of {pages}",
next: "Next",
},
validation: {
checking: "Checking file format…",
valid: "Format readable · content not verified",
invalid: "File validation failed · repair required",
unchecked: "File not checked",
recheck: "Recheck",
},
previewDialog: {
buttons: {
download: "Download",
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/i18n/locales/zh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1497,6 +1497,13 @@ const zh = {
page: "第 {page} 页,共 {pages} 页",
next: "下一页",
},
validation: {
checking: "正在检查文件格式…",
valid: "格式可读 · 内容尚未核验",
invalid: "文件校验失败 · 需要修复",
unchecked: "文件未校验",
recheck: "重新检查",
},
previewDialog: {
buttons: {
download: "下载",
Expand Down
5 changes: 5 additions & 0 deletions frontend/vitest.widget.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const widgetConfig = mergeConfig(baseConfig, defineConfig({
"src/components/file/file-preview-content.tsx",
"src/components/file/file-viewer.tsx",
"src/components/file/inline-file-preview.tsx",
"src/components/file/artifact-validation.tsx",
"src/components/file/pptx-preview-renderer.tsx",
"src/components/task/task-conversation-panel.tsx",
"src/components/ui/markdown-renderer.tsx",
Expand Down Expand Up @@ -116,6 +117,9 @@ const widgetConfig = mergeConfig(baseConfig, defineConfig({
"src/components/file/inline-file-preview.tsx": {
statements: 70, branches: 55, functions: 60, lines: 70,
},
"src/components/file/artifact-validation.tsx": {
statements: 90, branches: 80, functions: 90, lines: 90,
},
"src/components/file/pptx-preview-renderer.tsx": {
statements: 45, branches: 35, functions: 30, lines: 45,
},
Expand Down Expand Up @@ -148,6 +152,7 @@ export default defineConfig({
"src/components/file/file-preview-content.test.tsx",
"src/components/file/file-viewer.test.tsx",
"src/components/file/inline-file-preview.test.tsx",
"src/components/file/artifact-validation.test.tsx",
"src/components/file/pptx-preview-renderer.test.tsx",
"src/components/layout/sidebar.test.tsx",
"src/components/pages/login.test.tsx",
Expand Down
5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ dependencies = [
"celery[redis]>=5.4.0,<6.0.0",
"redis>=5.0.0",
"pillow >= 10.0.0",
"defusedxml>=0.7.1",
"pypinyin>=0.53.0",
"cairosvg >= 2.7.1",
"lancedb>=0.24.2",
Expand Down Expand Up @@ -298,9 +299,13 @@ module = [
"asyncssh",
"bashlex",
"docx",
"defusedxml",
"defusedxml.*",
"pptx",
"pptx.*",
"pdfplumber",
"pypdf",
"pypdf.*",
"unstructured.*",
"fitz",
"pandas",
Expand Down
24 changes: 24 additions & 0 deletions src/xagent/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1811,6 +1811,30 @@ def get_frontend_dist_dir() -> Path:
return get_web_dir() / "frontend_dist"


ARTIFACT_VALIDATION_MAX_BYTES = "XAGENT_ARTIFACT_VALIDATION_MAX_BYTES"
ARTIFACT_VALIDATION_TIMEOUT_SECONDS = "XAGENT_ARTIFACT_VALIDATION_TIMEOUT_SECONDS"


Comment thread
qinxuye marked this conversation as resolved.
def get_artifact_validation_max_bytes() -> int:
"""Maximum snapshot bytes to format-check (larger files remain unchecked)."""
value = int(os.getenv(ARTIFACT_VALIDATION_MAX_BYTES, str(32 * 1024 * 1024)))
if value <= 0:
raise ValueError(f"{ARTIFACT_VALIDATION_MAX_BYTES} must be positive")
return value


def get_artifact_validation_timeout_seconds() -> float:
"""Hard timeout for each isolated artifact parser process."""
import math

value = float(os.getenv(ARTIFACT_VALIDATION_TIMEOUT_SECONDS, "8"))
if not math.isfinite(value) or value <= 0:
raise ValueError(
f"{ARTIFACT_VALIDATION_TIMEOUT_SECONDS} must be positive and finite"
)
return value


def get_max_upload_size_bytes() -> int:
"""Get the maximum allowed upload size in bytes.

Expand Down
12 changes: 12 additions & 0 deletions src/xagent/core/artifact_validation/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
"""Modular artifact readability checks, independent of skills and agent patterns."""

from .models import ArtifactCheck, ArtifactContent, ValidationLimits, ValidationReport
from .registry import ArtifactCheckRegistry

__all__ = [
"ArtifactCheck",
"ArtifactCheckRegistry",
"ArtifactContent",
"ValidationLimits",
"ValidationReport",
]
Loading