diff --git a/docker-compose.yml b/docker-compose.yml index 378c01bbbf..05be0bf15d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,6 +41,8 @@ services: - BACKEND_PORT=8000 - XAGENT_UPLOADS_DIR=/root/.xagent/uploads - XAGENT_MAX_UPLOAD_SIZE=${XAGENT_MAX_UPLOAD_SIZE:-100M} + - XAGENT_ARTIFACT_VALIDATION_MAX_BYTES=${XAGENT_ARTIFACT_VALIDATION_MAX_BYTES:-32M} + - XAGENT_ARTIFACT_VALIDATION_TIMEOUT_SECONDS=${XAGENT_ARTIFACT_VALIDATION_TIMEOUT_SECONDS:-8} - XAGENT_REDIS_URL=redis://redis:6379/0 - XAGENT_HOT_PATH_CACHE_ENABLED=${XAGENT_HOT_PATH_CACHE_ENABLED:-true} - XAGENT_CELERY_ENABLED=${XAGENT_CELERY_ENABLED:-true} @@ -92,6 +94,8 @@ services: - POSTGRES_PASSWORD=${POSTGRES_PASSWORD:-xagent_password} - XAGENT_UPLOADS_DIR=/root/.xagent/uploads - XAGENT_MAX_UPLOAD_SIZE=${XAGENT_MAX_UPLOAD_SIZE:-100M} + - XAGENT_ARTIFACT_VALIDATION_MAX_BYTES=${XAGENT_ARTIFACT_VALIDATION_MAX_BYTES:-32M} + - XAGENT_ARTIFACT_VALIDATION_TIMEOUT_SECONDS=${XAGENT_ARTIFACT_VALIDATION_TIMEOUT_SECONDS:-8} - XAGENT_REDIS_URL=redis://redis:6379/0 - XAGENT_HOT_PATH_CACHE_ENABLED=${XAGENT_HOT_PATH_CACHE_ENABLED:-true} - XAGENT_CELERY_ENABLED=${XAGENT_CELERY_ENABLED:-true} diff --git a/example.env b/example.env index 2a48e7a8a7..345fb256d0 100644 --- a/example.env +++ b/example.env @@ -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=32M +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. diff --git a/frontend/src/components/file/artifact-validation.test.tsx b/frontend/src/components/file/artifact-validation.test.tsx new file mode 100644 index 0000000000..280d489425 --- /dev/null +++ b/frontend/src/components/file/artifact-validation.test.tsx @@ -0,0 +1,116 @@ +/// +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 = {}) => + {children} + + +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(artifact, 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(file, 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(file, 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) => file + 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(file, 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(file) + 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(, 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(file, 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(file, 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(file, request)) + await screen.findByText('文件校验失败 · 需要修复') + expect(screen.queryByText('PDF header is missing.')).toBeNull() + expect(screen.getByRole('button', { name: '重新检查' })).toBeInTheDocument() + }) +}) diff --git a/frontend/src/components/file/artifact-validation.tsx b/frontend/src/components/file/artifact-validation.tsx new file mode 100644 index 0000000000..80d19c0e9e --- /dev/null +++ b/frontend/src/components/file/artifact-validation.tsx @@ -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 = new Set(['valid', 'invalid', 'unchecked']) + +async function readReport(response: Response): Promise { + 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() + 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 ( +
+
+ + {t(`files.validation.${label}`)} + + {current ? ( + + ) : null} +
+ {children} +
+ ) +} diff --git a/frontend/src/components/file/inline-file-preview.test.tsx b/frontend/src/components/file/inline-file-preview.test.tsx index 7c5ec6d611..5ea8f7e5e2 100644 --- a/frontend/src/components/file/inline-file-preview.test.tsx +++ b/frontend/src/components/file/inline-file-preview.test.tsx @@ -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 }, })) diff --git a/frontend/src/components/file/inline-file-preview.tsx b/frontend/src/components/file/inline-file-preview.tsx index 3529a702cf..5799e6e57d 100644 --- a/frontend/src/components/file/inline-file-preview.tsx +++ b/frontend/src/components/file/inline-file-preview.tsx @@ -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' @@ -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 = + // 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 {content} +} + +function InlineFilePreviewContent({ source, className, imageClassName, diff --git a/frontend/src/contexts/file-access-context.tsx b/frontend/src/contexts/file-access-context.tsx index 94951bd4ec..d88134b82f 100644 --- a/frontend/src/contexts/file-access-context.tsx +++ b/frontend/src/contexts/file-access-context.tsx @@ -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 @@ -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" }) @@ -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), diff --git a/frontend/src/i18n/locales/en.ts b/frontend/src/i18n/locales/en.ts index 049347c754..b75c6c3c86 100644 --- a/frontend/src/i18n/locales/en.ts +++ b/frontend/src/i18n/locales/en.ts @@ -1497,6 +1497,14 @@ 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", + error: "Unable to request file validation · try again", + recheck: "Recheck", + }, previewDialog: { buttons: { download: "Download", diff --git a/frontend/src/i18n/locales/zh.ts b/frontend/src/i18n/locales/zh.ts index 1c16aaff9a..f0ea6336d5 100644 --- a/frontend/src/i18n/locales/zh.ts +++ b/frontend/src/i18n/locales/zh.ts @@ -1497,6 +1497,14 @@ const zh = { page: "第 {page} 页,共 {pages} 页", next: "下一页", }, + validation: { + checking: "正在检查文件格式…", + valid: "格式可读 · 内容尚未核验", + invalid: "文件校验失败 · 需要修复", + unchecked: "文件未校验", + error: "无法请求文件校验 · 请重试", + recheck: "重新检查", + }, previewDialog: { buttons: { download: "下载", diff --git a/frontend/vitest.widget.config.ts b/frontend/vitest.widget.config.ts index 4f62f648b3..ada016b0e9 100644 --- a/frontend/vitest.widget.config.ts +++ b/frontend/vitest.widget.config.ts @@ -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", @@ -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, }, @@ -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", diff --git a/pyproject.toml b/pyproject.toml index 901fbde9b4..3ee8e8af83 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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", @@ -298,9 +299,13 @@ module = [ "asyncssh", "bashlex", "docx", + "defusedxml", + "defusedxml.*", "pptx", "pptx.*", "pdfplumber", + "pypdf", + "pypdf.*", "unstructured.*", "fitz", "pandas", diff --git a/src/xagent/config.py b/src/xagent/config.py index bcea7ac5a9..81453ff3d8 100644 --- a/src/xagent/config.py +++ b/src/xagent/config.py @@ -1811,6 +1811,29 @@ 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" + + +def get_artifact_validation_max_bytes() -> int: + """Maximum snapshot bytes to format-check (larger files remain unchecked).""" + return _parse_size_bytes( + os.getenv(ARTIFACT_VALIDATION_MAX_BYTES) or "32M", ARTIFACT_VALIDATION_MAX_BYTES + ) + + +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) or "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. @@ -1833,9 +1856,14 @@ def get_max_upload_size_bytes() -> int: if not env_value: return 100 * 1024 * 1024 - normalized = env_value.strip().upper() - if not normalized: + if not env_value.strip(): return 100 * 1024 * 1024 + return _parse_size_bytes(env_value, MAX_UPLOAD_SIZE) + + +def _parse_size_bytes(env_value: str, setting: str) -> int: + """Shared positive byte-size parser for upload and validation budgets.""" + normalized = env_value.strip().upper() suffix_multipliers = [ ("GB", 1024 * 1024 * 1024), @@ -1853,27 +1881,23 @@ def get_max_upload_size_bytes() -> int: number_part = normalized[: -len(suffix)].strip() if not number_part: raise ValueError( - f"Invalid {MAX_UPLOAD_SIZE} value: {env_value!r}. Missing numeric value." + f"Invalid {setting} value: {env_value!r}. Missing numeric value." ) try: result = int(float(number_part) * multiplier) - except ValueError as exc: - raise ValueError( - f"Invalid {MAX_UPLOAD_SIZE} value: {env_value!r}." - ) from exc + except (ValueError, OverflowError) as exc: + raise ValueError(f"Invalid {setting} value: {env_value!r}.") from exc break if result is None: try: result = int(float(normalized)) - except ValueError as exc: - raise ValueError( - f"Invalid {MAX_UPLOAD_SIZE} value: {env_value!r}." - ) from exc + except (ValueError, OverflowError) as exc: + raise ValueError(f"Invalid {setting} value: {env_value!r}.") from exc if result <= 0: raise ValueError( - f"Invalid {MAX_UPLOAD_SIZE} value: {env_value!r}. Value must be positive." + f"Invalid {setting} value: {env_value!r}. Value must be positive." ) return result diff --git a/src/xagent/core/artifact_validation/__init__.py b/src/xagent/core/artifact_validation/__init__.py new file mode 100644 index 0000000000..dc9a3ae392 --- /dev/null +++ b/src/xagent/core/artifact_validation/__init__.py @@ -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", +] diff --git a/src/xagent/core/artifact_validation/defaults.py b/src/xagent/core/artifact_validation/defaults.py new file mode 100644 index 0000000000..aa582cafba --- /dev/null +++ b/src/xagent/core/artifact_validation/defaults.py @@ -0,0 +1,26 @@ +"""Composition root. Add checks here without modifying tools or delivery code.""" + +from .formats import check_csv, check_image, check_pdf +from .models import ArtifactCheck +from .office import check_office_document, check_package +from .registry import ArtifactCheckRegistry + + +def default_registry() -> ArtifactCheckRegistry: + registry = ArtifactCheckRegistry() + office = frozenset({".xlsx", ".docx", ".pptx"}) + for check in ( + ArtifactCheck("office-package", office, check_package), + ArtifactCheck("office-reader", office, check_office_document), + ArtifactCheck("csv-reader", frozenset({".csv", ".tsv"}), check_csv), + ArtifactCheck("pdf-reader", frozenset({".pdf"}), check_pdf), + ArtifactCheck( + "image-decoder", + frozenset( + {".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tif", ".tiff"} + ), + check_image, + ), + ): + registry.register(check) + return registry diff --git a/src/xagent/core/artifact_validation/formats.py b/src/xagent/core/artifact_validation/formats.py new file mode 100644 index 0000000000..cf88126e80 --- /dev/null +++ b/src/xagent/core/artifact_validation/formats.py @@ -0,0 +1,142 @@ +"""Independent format checks; no business-content or minimum-size heuristics.""" + +import csv +import zlib +from io import BytesIO, StringIO + +from .models import ArtifactContent, InvalidArtifact, UncheckedArtifact + + +def check_csv(content: ArtifactContent) -> None: + data = content.data + try: + # BOMs are explicit encoding declarations. Do not guess latin-1 and + # thereby accept arbitrary binary files as a one-column CSV. + encoding = ( + "utf-16" if data.startswith((b"\xff\xfe", b"\xfe\xff")) else "utf-8-sig" + ) + text = data.decode(encoding) + except UnicodeError as exc: + raise UncheckedArtifact( + "CSV encoding is not UTF-8 or BOM-declared UTF-16." + ) from exc + if "\x00" in text: + raise InvalidArtifact("CSV contains binary NUL characters.") + try: + dialect = csv.Sniffer().sniff(text[:8192], delimiters=",;\t|") + except csv.Error: + dialect = csv.excel + try: + csv.field_size_limit(content.limits.max_bytes) + for index, _row in enumerate( + csv.reader(StringIO(text, newline=""), dialect, strict=True) + ): + if index >= content.limits.max_units: + raise UncheckedArtifact("CSV exceeds the row budget.") + # Ragged rows, an empty file, or a single column can all be legitimate. + # Schema/business expectations belong to additional explicit checks. + except csv.Error as exc: + # Lenient readers may recover nonstandard quoting, but can also merge + # truncated records silently. Neither corruption nor validity is proven. + raise UncheckedArtifact( + "CSV quoting or record structure is ambiguous." + ) from exc + + +def check_pdf(content: ArtifactContent) -> None: + from pypdf import PdfReader + from pypdf.errors import DependencyError, PdfReadError, PyPdfError + from pypdf.generic import ArrayObject, EncodedStreamObject + + if not content.data.lstrip().startswith(b"%PDF-"): + raise InvalidArtifact("PDF header is missing.") + try: + # Validate readability, not strict PDF conformance. Real readers can + # recover common producer defects such as an inaccurate xref offset. + reader = PdfReader(BytesIO(content.data), strict=False) + if reader.is_encrypted: + raise UncheckedArtifact("Encrypted PDFs require a password to validate.") + # A fixed parser ceiling, not partial validation: larger PDFs are + # unchecked in their entirety, never certified from a page sample. + page_limit = min(content.limits.max_units, 500) + if len(reader.pages) > page_limit: + raise UncheckedArtifact( + f"PDF exceeds the page budget ({page_limit} pages)." + ) + expanded = 0 + for page in reader.pages: + raw_contents = page.get("/Contents") + if raw_contents is not None: + raw_contents = raw_contents.get_object() + parts = ( + raw_contents + if isinstance(raw_contents, ArrayObject) + else [raw_contents] + ) + for part in parts: + part = part.get_object() + if not isinstance(part, EncodedStreamObject): + continue + filters = part.get("/Filter") + if filters not in ( + "/FlateDecode", + "/Fl", + ["/FlateDecode"], + ["/Fl"], + ): + continue + # pypdf's recovery decoder can silently return empty bytes + # for an unreadable FlateDecode stream. Distinguish that + # from a genuinely empty compressed stream, without treating + # successful nonempty recovery as strict-conformance failure. + if not part.get_data(): + encoded = getattr(part, "_data", None) + if not isinstance(encoded, bytes): + raise UncheckedArtifact( + "PDF reader does not expose encoded stream bytes for recovery verification." + ) + try: + decoder = zlib.decompressobj() + decoded = decoder.decompress(encoded, 1) + except zlib.error as exc: + raise UncheckedArtifact( + "PDF content stream recovery could not be verified." + ) from exc + if decoded or not decoder.eof: + raise UncheckedArtifact( + "PDF content stream recovery could not be verified." + ) + stream = page.get_contents() + if stream is not None: + expanded += len(stream.get_data()) + if expanded > content.limits.max_expanded_bytes: + raise UncheckedArtifact("PDF exceeds the decoded content budget.") + except (PdfReadError, ValueError, KeyError, TypeError, OSError) as exc: + raise InvalidArtifact( + "PDF structure or page content cannot be decoded." + ) from exc + except (PyPdfError, DependencyError) as exc: + raise UncheckedArtifact( + "PDF reader could not complete validation (dependency or parser limit)." + ) from exc + + +def check_image(content: ArtifactContent) -> None: + from PIL import Image, UnidentifiedImageError + + try: + with Image.open(BytesIO(content.data)) as image: + width, height = image.size + frames = getattr(image, "n_frames", 1) + if width * height * frames > content.limits.max_pixels: + raise UncheckedArtifact("Image exceeds the decoded pixel budget.") + image.verify() + # verify() checks container structure; load() also exercises decoding. + with Image.open(BytesIO(content.data)) as image: + for index in range(getattr(image, "n_frames", 1)): + image.seek(index) + image.load() + except Image.DecompressionBombError as exc: + raise UncheckedArtifact("Image exceeds the decoded pixel budget.") from exc + except (UnidentifiedImageError, OSError, ValueError, SyntaxError) as exc: + raise InvalidArtifact("Image data cannot be decoded.") from exc diff --git a/src/xagent/core/artifact_validation/models.py b/src/xagent/core/artifact_validation/models.py new file mode 100644 index 0000000000..ef4f4a571d --- /dev/null +++ b/src/xagent/core/artifact_validation/models.py @@ -0,0 +1,69 @@ +"""Transport-neutral contracts for bounded, format-specific artifact checks.""" + +from dataclasses import dataclass +from typing import Any, Callable, Literal + +ValidationStatus = Literal["valid", "invalid", "unchecked"] + + +class InvalidArtifact(Exception): + """A known format error; the message must be safe for user/model output.""" + + +class UncheckedArtifact(Exception): + """The check cannot establish validity (dependency, budget, encryption).""" + + +@dataclass(frozen=True) +class ValidationLimits: + max_bytes: int = 32 * 1024 * 1024 + max_expanded_bytes: int = 64 * 1024 * 1024 + max_entries: int = 2048 + max_units: int = 200_000 + max_pixels: int = 25_000_000 + + +@dataclass(frozen=True) +class ArtifactContent: + """An immutable snapshot. Checks must not read paths or change files.""" + + filename: str + data: bytes + limits: ValidationLimits + + +@dataclass(frozen=True) +class ArtifactCheck: + name: str + extensions: frozenset[str] + run: Callable[[ArtifactContent], None] + + +@dataclass(frozen=True) +class CheckResult: + name: str + status: ValidationStatus + message: str + + +@dataclass(frozen=True) +class ValidationReport: + status: ValidationStatus + checks: tuple[CheckResult, ...] + sha256: str | None = None + + def as_dict(self) -> dict[str, Any]: + return { + "status": self.status, + "sha256": self.sha256, + "checks": [ + {"name": c.name, "status": c.status, "message": c.message} + for c in self.checks + ], + } + + +def unchecked(message: str, *, sha256: str | None = None) -> ValidationReport: + return ValidationReport( + "unchecked", (CheckResult("availability", "unchecked", message),), sha256 + ) diff --git a/src/xagent/core/artifact_validation/office.py b/src/xagent/core/artifact_validation/office.py new file mode 100644 index 0000000000..2f2b286645 --- /dev/null +++ b/src/xagent/core/artifact_validation/office.py @@ -0,0 +1,96 @@ +"""OOXML package preflight, followed by the actual document readers.""" + +from io import BytesIO +from pathlib import Path, PurePosixPath +from zipfile import BadZipFile, ZipFile + +from .models import ArtifactContent, InvalidArtifact, UncheckedArtifact + + +def check_package(content: ArtifactContent) -> None: + from xml.etree.ElementTree import ParseError + + from defusedxml.common import DefusedXmlException + from defusedxml.ElementTree import fromstring + + try: + with ZipFile(BytesIO(content.data)) as archive: + entries = archive.infolist() + if ( + len(entries) > content.limits.max_entries + or sum(e.file_size for e in entries) > content.limits.max_expanded_bytes + ): + raise UncheckedArtifact("Office package exceeds the expansion budget.") + names = [e.filename for e in entries] + if len(names) != len(set(names)) or any( + PurePosixPath(n).is_absolute() or ".." in PurePosixPath(n).parts + for n in names + ): + raise InvalidArtifact("Office package contains ambiguous member paths.") + # Main-part locations come from content types and relationships, + # not conventional filenames. The format reader resolves them + # after this package-wide safety preflight has passed. + required = {"[Content_Types].xml", "_rels/.rels"} + if not required.issubset(names): + raise InvalidArtifact( + "Office package is missing required document parts." + ) + expanded = 0 + for entry in entries: + if entry.flag_bits & 1: + raise UncheckedArtifact( + "Encrypted Office packages cannot be checked." + ) + # Read all members to exercise CRC/truncation checks, without + # extracting files or trusting compressed-size claims alone. + with archive.open(entry) as stream: + data = stream.read(content.limits.max_expanded_bytes - expanded + 1) + expanded += len(data) + if expanded > content.limits.max_expanded_bytes: + raise UncheckedArtifact( + "Office package exceeds the expansion budget." + ) + if entry.filename.endswith((".xml", ".rels")): + fromstring(data) + except (BadZipFile, ParseError, DefusedXmlException, EOFError, OSError) as exc: + raise InvalidArtifact( + "Office package is corrupt or contains unsafe XML." + ) from exc + + +def check_office_document(content: ArtifactContent) -> None: + suffix = Path(content.filename).suffix.lower() + try: + if suffix == ".xlsx": + from openpyxl import load_workbook + + workbook = load_workbook( + BytesIO(content.data), read_only=True, data_only=False, keep_links=False + ) + try: + cells = 0 + for sheet in workbook: + # Ignore stale dimension metadata; walk actual cells. Empty + # workbooks/templates are legal, not business failures. + sheet.reset_dimensions() + for row in sheet.iter_rows(): + cells += len(row) + if cells > content.limits.max_units: + raise UncheckedArtifact("Workbook exceeds the cell budget.") + finally: + workbook.close() + elif suffix == ".docx": + from docx import Document + + document = Document(BytesIO(content.data)) + _ = document.paragraphs, document.tables + else: + from pptx import Presentation + + presentation = Presentation(BytesIO(content.data)) + for slide in presentation.slides: + _ = slide.shapes + except (ValueError, KeyError, IndexError, TypeError, OSError, BadZipFile) as exc: + raise InvalidArtifact( + "Office document cannot be opened by its format reader." + ) from exc diff --git a/src/xagent/core/artifact_validation/registry.py b/src/xagent/core/artifact_validation/registry.py new file mode 100644 index 0000000000..7e31bfaa29 --- /dev/null +++ b/src/xagent/core/artifact_validation/registry.py @@ -0,0 +1,82 @@ +"""Checks compose by extension; delivery callers do not know about formats.""" + +import hashlib +import logging +from pathlib import Path + +from .models import ( + ArtifactCheck, + ArtifactContent, + CheckResult, + InvalidArtifact, + UncheckedArtifact, + ValidationReport, + unchecked, +) + +logger = logging.getLogger(__name__) + + +class ArtifactCheckRegistry: + def __init__(self) -> None: + self._checks: dict[str, ArtifactCheck] = {} + + def register(self, check: ArtifactCheck) -> None: + if not check.name or check.name in self._checks: + raise ValueError(f"Duplicate or empty artifact check name: {check.name}") + if not check.extensions or any( + not ext.startswith(".") or ext != ext.lower() for ext in check.extensions + ): + raise ValueError("Check extensions must be nonempty lowercase suffixes") + self._checks[check.name] = check + + def supports(self, filename: str) -> bool: + suffix = Path(filename).suffix.lower() + return any(suffix in check.extensions for check in self._checks.values()) + + @property + def extensions(self) -> frozenset[str]: + return frozenset( + ext for check in self._checks.values() for ext in check.extensions + ) + + def validate(self, content: ArtifactContent) -> ValidationReport: + digest = hashlib.sha256(content.data).hexdigest() + if len(content.data) > content.limits.max_bytes: + return unchecked("File exceeds the validation byte budget.", sha256=digest) + suffix = Path(content.filename).suffix.lower() + checks = [c for c in self._checks.values() if suffix in c.extensions] + if not checks: + return unchecked( + "No validator is installed for this format.", sha256=digest + ) + results = [] + for check in checks: + try: + check.run(content) + except InvalidArtifact as exc: + results.append(CheckResult(check.name, "invalid", str(exc))) + except UncheckedArtifact as exc: + results.append(CheckResult(check.name, "unchecked", str(exc))) + except ImportError: + results.append( + CheckResult( + check.name, "unchecked", "Parser dependency is unavailable." + ) + ) + except Exception: + # A checker bug is not evidence of a corrupt user file. Never + # expose parser exceptions (which can contain paths/content). + logger.exception("Artifact check %s failed unexpectedly", check.name) + results.append( + CheckResult( + check.name, "unchecked", "Validator could not complete." + ) + ) + else: + results.append(CheckResult(check.name, "valid", "Format check passed.")) + if results[-1].status != "valid": + # Structural/budget preflights run before expensive decoders. + break + status = results[-1].status + return ValidationReport(status, tuple(results), digest) diff --git a/src/xagent/core/artifact_validation/service.py b/src/xagent/core/artifact_validation/service.py new file mode 100644 index 0000000000..7c9ea6d191 --- /dev/null +++ b/src/xagent/core/artifact_validation/service.py @@ -0,0 +1,164 @@ +"""Bounded host-side validation, cached by bytes rather than a mutable filename. + +Authorization/path ownership remains with the caller. This module neither +registers nor deletes files and must never run inside a database transaction. +""" + +import hashlib +import json +import logging +import os +import stat +import subprocess +import sys +from collections import OrderedDict +from pathlib import Path +from threading import BoundedSemaphore, Lock + +from ...config import ( + get_artifact_validation_max_bytes, + get_artifact_validation_timeout_seconds, + in_sandbox_tool_runner, +) +from .defaults import default_registry +from .models import CheckResult, ValidationReport, unchecked + +logger = logging.getLogger(__name__) + +_slots = BoundedSemaphore(2) +# Public capability URLs may be shared widely. Reserve at least one worker +# slot for authenticated/tool callers and never queue public requests here. +_public_slots = BoundedSemaphore(1) +_cache_lock = Lock() +_cache: OrderedDict[tuple[str, str, int], ValidationReport] = OrderedDict() + + +def _run_checks( + filename: str, data: bytes, max_bytes: int, timeout: float +) -> ValidationReport: + try: + # A subprocess timeout actually terminates a stuck parser; timing out a + # thread would leave it consuming resources. Bytes, not paths, cross + # this boundary, so the report describes exactly the parent snapshot. + completed = subprocess.run( + [ + sys.executable, + "-m", + "xagent.core.artifact_validation.worker", + filename, + str(max_bytes), + ], + input=data, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + timeout=timeout, + check=True, + env={**os.environ, "OPENBLAS_NUM_THREADS": "1"}, + ) + value = json.loads(completed.stdout) + statuses = {"valid", "invalid", "unchecked"} + if value["status"] not in statuses or not value["checks"]: + raise ValueError("Invalid validation report") + checks = tuple(CheckResult(**item) for item in value["checks"]) + if any(c.status not in statuses for c in checks): + raise ValueError("Invalid check status") + return ValidationReport(value["status"], checks, value["sha256"]) + except subprocess.TimeoutExpired: + return unchecked("File validation exceeded its time budget.") + except (subprocess.SubprocessError, OSError, ValueError, KeyError, TypeError): + logger.exception("Artifact validator process failed unexpectedly") + return unchecked("Validator process could not complete.") + + +def validate_artifact( + path: str | Path, *, filename: str | None = None, public: bool = False +) -> ValidationReport: + if in_sandbox_tool_runner(): + return unchecked("Awaiting host-side file validation.") + path = Path(path) + filename = Path(filename or path.name).name + if not default_registry().supports(filename): + return unchecked("No validator is installed for this format.") + try: + max_bytes = get_artifact_validation_max_bytes() + timeout = get_artifact_validation_timeout_seconds() + except ValueError: + logger.warning("Invalid artifact validation configuration; checks are disabled") + return unchecked("Validation configuration is invalid.") + if public and not _public_slots.acquire(blocking=False): + return unchecked("Public validation capacity is busy; retry later.") + # Acquire before loading bytes, not merely before launching the parser. + # Concurrent preview requests must not each retain a max-sized snapshot. + try: + if not _slots.acquire(timeout=timeout): + return unchecked("Validation capacity is busy; file has not been checked.") + try: + try: + return _validate_snapshot(path, filename, max_bytes, timeout) + except Exception: + # Validation is advisory: snapshot/host failures must not turn + # a successfully registered output into a failed tool result. + logger.exception("Artifact snapshot validation failed unexpectedly") + return unchecked("File validation could not complete.") + finally: + _slots.release() + finally: + if public: + _public_slots.release() + + +def _validate_snapshot( + path: Path, filename: str, max_bytes: int, timeout: float +) -> ValidationReport: + try: + if path.stat().st_size > max_bytes: + return unchecked("File exceeds the validation byte budget.") + # Nonblocking open prevents a replaced file/FIFO from hanging before + # the killable parser process even starts. Validate the opened inode. + with os.fdopen( + os.open(path, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0)), "rb" + ) as stream: + before = os.fstat(stream.fileno()) + if not stat.S_ISREG(before.st_mode): + return unchecked("Only regular files can be validated.") + data = stream.read(max_bytes + 1) + after = os.fstat(stream.fileno()) + current = path.stat() + except OSError: + return unchecked("File bytes are unavailable for validation.") + if len(data) > max_bytes: + return unchecked("File exceeds the validation byte budget.") + + def identity(stat_result: os.stat_result) -> tuple[int, ...]: + return ( + stat_result.st_dev, + stat_result.st_ino, + stat_result.st_size, + stat_result.st_mtime_ns, + stat_result.st_ctime_ns, + ) + + if identity(before) != identity(after) or identity(after) != identity(current): + return unchecked("File changed during validation; check its new version.") + digest = hashlib.sha256(data).hexdigest() + key = (digest, filename, max_bytes) + with _cache_lock: + cached = _cache.get(key) + if cached: + _cache.move_to_end(key) + report = cached or _run_checks(filename, data, max_bytes, timeout) + try: + if identity(path.stat()) != identity(after): + return unchecked("File changed during validation; check its new version.") + except OSError: + return unchecked("File bytes are unavailable for validation.") + if report.sha256 not in (None, digest): + return unchecked("Validation report did not match the file snapshot.") + report = ValidationReport(report.status, report.checks, digest) + if report.status != "unchecked": + with _cache_lock: + _cache[key] = report + _cache.move_to_end(key) + while len(_cache) > 128: + _cache.popitem(last=False) + return report diff --git a/src/xagent/core/artifact_validation/worker.py b/src/xagent/core/artifact_validation/worker.py new file mode 100644 index 0000000000..443cc36a12 --- /dev/null +++ b/src/xagent/core/artifact_validation/worker.py @@ -0,0 +1,37 @@ +"""Short-lived parser process. Input bytes are snapshotted by the parent.""" + +import json +import sys + +from .defaults import default_registry +from .models import ArtifactContent, ValidationLimits + + +def main() -> None: + # Bound decoder allocations on Linux in addition to parent-enforced wall + # time and per-format expansion limits. macOS does not enforce RLIMIT_AS + # consistently, so retain the portable input/expansion/pixel budgets there. + if sys.platform == "linux": + import resource + + soft, hard = resource.getrlimit(resource.RLIMIT_AS) + resource.setrlimit( + resource.RLIMIT_AS, + ( + 1024**3 if soft == resource.RLIM_INFINITY else min(soft, 1024**3), + 1024**3 if hard == resource.RLIM_INFINITY else min(hard, 1024**3), + ), + ) + limits = ValidationLimits(max_bytes=int(sys.argv[2])) + # Parser libraries may write warnings to stdout. Keep the protocol output + # separate from their diagnostics. + output = sys.stdout + sys.stdout = sys.stderr + data = sys.stdin.buffer.read(limits.max_bytes + 1) + report = default_registry().validate(ArtifactContent(sys.argv[1], data, limits)) + output.write(json.dumps(report.as_dict())) + output.flush() + + +if __name__ == "__main__": + main() diff --git a/src/xagent/core/file_ref.py b/src/xagent/core/file_ref.py index a00aa9d1ea..972ae1eb50 100644 --- a/src/xagent/core/file_ref.py +++ b/src/xagent/core/file_ref.py @@ -75,6 +75,10 @@ def final_deliverable_file_reference_instructions( - When the user requests a new file or file-based artifact, it is not delivered until a successful tool result returns its registered FileRef or markdown_link. - Do not call final_answer claiming that a file was created or delivered unless that result exists.""" +FILE_REF_OUTPUT_INSTRUCTIONS += """ +- Tool execution success and file validation are separate. If validation.status is invalid, repair the file and recheck it, or clearly report the failure; do not present it as a usable completed deliverable. Keep its file_id available for repair. +- An unchecked or absent validation result is not a pass. State that limitation when delivering the file; a valid result establishes format readability only, not business/content correctness.""" + FILE_REF_MODEL_INSTRUCTIONS = f"""## FILE REFERENCES Files are referenced by FileRef objects. Treat file_id as the canonical file handle. @@ -192,12 +196,17 @@ def build_workspace_file_ref( file_id: str | None = None, mime_type: str | None = None, internal: bool = False, + validate: bool = False, ) -> dict[str, Any]: """Register a workspace file and build the model/API-facing FileRef. ``internal`` keeps execution scratch data resolvable by ``file_id`` without creating a user-visible or durably uploaded file record. It fails closed when the workspace does not support internal registration. + + ``validate`` checks a completed output snapshot without changing registration + or tool success. Call from a worker thread, as with other blocking file I/O. + It applies only to user-visible outputs; internal scratch files are not checked. """ resolved_path = Path(file_path).resolve() if not resolved_path.exists() or not resolved_path.is_file(): @@ -233,6 +242,10 @@ def build_workspace_file_ref( except ValueError: relative_path = str(resolved_path) result["relative_path"] = relative_path + if validate: + from .artifact_validation.service import validate_artifact + + result["validation"] = validate_artifact(resolved_path).as_dict() return result diff --git a/src/xagent/core/tools/adapters/vibe/sandboxed_tool/sandboxed_tool_wrapper.py b/src/xagent/core/tools/adapters/vibe/sandboxed_tool/sandboxed_tool_wrapper.py index 654f3567c8..502b2f8770 100644 --- a/src/xagent/core/tools/adapters/vibe/sandboxed_tool/sandboxed_tool_wrapper.py +++ b/src/xagent/core/tools/adapters/vibe/sandboxed_tool/sandboxed_tool_wrapper.py @@ -493,6 +493,16 @@ def run_json_sync(self, args: Mapping[str, Any]) -> Any: async def run_json_async(self, args: Mapping[str, Any]) -> Any: """Execute the tool in the sandbox, then register its files here.""" result = await self._run_json_in_sandbox(args) + # Guest metadata is not evidence about persisted host bytes. Only the + # host rebuild below can attach an authoritative validation report. + pending = [result] + while pending: + value = pending.pop() + if isinstance(value, dict): + value.pop("validation", None) + pending.extend(value.values()) + elif isinstance(value, list): + pending.extend(value) try: return await self._register_sandbox_outputs(result) except Exception: diff --git a/src/xagent/core/tools/artifacts.py b/src/xagent/core/tools/artifacts.py index a95153f150..04bef4458f 100644 --- a/src/xagent/core/tools/artifacts.py +++ b/src/xagent/core/tools/artifacts.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import Any, Iterable +from ..artifact_validation.defaults import default_registry from ..file_ref import ( build_workspace_file_ref, guess_mime_type, @@ -33,7 +34,7 @@ ".webm", ".xls", ".xlsx", -} +} | set(default_registry().extensions) SAFE_FILE_REF_KEYS = { "download_url", "file_id", @@ -44,6 +45,7 @@ "preview_url", "relative_path", "size", + "validation", } # Every media tool returns its artifact as _path alongside file_id/file_ref. # Missing keys were redacted only by coincidence: file_ref.file_path registers the @@ -100,7 +102,7 @@ def artifact_type_for_filename(filename: str) -> str: return "file" -def build_inline_artifact(file_ref: dict[str, Any]) -> dict[str, str]: +def build_inline_artifact(file_ref: dict[str, Any]) -> dict[str, Any]: filename = str(file_ref.get("filename") or "artifact") return { "type": artifact_type_for_filename(filename), @@ -108,6 +110,7 @@ def build_inline_artifact(file_ref: dict[str, Any]) -> dict[str, str]: "filename": filename, "mime_type": str(file_ref.get("mime_type") or guess_mime_type(filename)), "display": "inline", + **({"validation": file_ref["validation"]} if "validation" in file_ref else {}), } @@ -148,7 +151,7 @@ def format_tool_result_for_observation(tool_name: str, result: Any) -> str: metadata = _observation_metadata(sanitized, _OBSERVATION_EXCLUDED_KEYS) return ( - f"Tool '{tool_name}' produced displayable artifact(s):\n" + f"Tool '{tool_name}' produced artifact(s); check validation before delivery:\n" + "\n".join(artifact_lines) + "\nUse the Markdown/chat form in assistant messages. " + "When writing HTML for Xagent preview, reference the same file_id " @@ -179,7 +182,7 @@ def _format_artifact_lines(artifacts: list[Any]) -> list[str]: [ f"- {filename}", " file_id: unavailable, registration did not complete", - " The file itself is written and intact. No file_id " + " The tool reported a written file; this does not establish readability. No file_id " "can be obtained for it in this task; say so plainly " "and never rewrite the file to try to mint one.", ] @@ -200,6 +203,33 @@ def _format_artifact_lines(artifacts: list[Any]) -> list[str]: ] ) ) + validation = artifact.get("validation") + if not isinstance(validation, dict): + lines.append( + " Validation: NOT RUN. No validation report was produced for this file." + ) + continue + status = validation.get("status", "unchecked") + if status == "invalid": + lines.append( + " Validation: INVALID. Repair and recheck, or report failure; do not claim a usable deliverable." + ) + elif status == "valid": + lines.append( + " Validation: format readable; content correctness is not checked." + ) + else: + lines.append( + " Validation: UNCHECKED. Do not claim this file passed validation." + ) + checks = validation.get("checks") + if isinstance(checks, list): + for check in checks: + if isinstance(check, dict) and check.get("status") == "unchecked": + message = check.get("message") + if isinstance(message, str) and message.strip(): + lines.append(f" Validation reason: {message}") + break return lines @@ -304,7 +334,7 @@ def build_generated_file_metadata( file_paths: Iterable[str | Path], ) -> dict[str, list[Any]]: file_refs: list[dict[str, Any]] = [] - artifacts: list[dict[str, str]] = [] + artifacts: list[dict[str, Any]] = [] generated_files: list[str] = [] for file_path in sorted({Path(path).resolve() for path in file_paths}): @@ -314,7 +344,7 @@ def build_generated_file_metadata( continue try: file_ref = build_workspace_file_ref( - workspace=workspace, file_path=file_path + workspace=workspace, file_path=file_path, validate=True ) except Exception as exc: # noqa: BLE001 logger.warning( diff --git a/src/xagent/core/tools/core/workspace_file_tool.py b/src/xagent/core/tools/core/workspace_file_tool.py index 51278ad6ff..e3d7d766a3 100644 --- a/src/xagent/core/tools/core/workspace_file_tool.py +++ b/src/xagent/core/tools/core/workspace_file_tool.py @@ -364,6 +364,7 @@ def write_file( file_ref = build_workspace_file_ref( workspace=self.workspace, file_path=resolved_path, + validate=True, ) logger.debug( @@ -381,6 +382,7 @@ def _registered_write_result(self, file_path: Path) -> Dict[str, Any]: file_ref = build_workspace_file_ref( workspace=self.workspace, file_path=file_path, + validate=True, ) return { "success": True, diff --git a/src/xagent/web/api/files.py b/src/xagent/web/api/files.py index ed1cc0def1..ba71cd1f6e 100644 --- a/src/xagent/web/api/files.py +++ b/src/xagent/web/api/files.py @@ -19,6 +19,7 @@ ) from fastapi.responses import ( FileResponse, + JSONResponse, RedirectResponse, Response, ) @@ -36,6 +37,8 @@ get_storage_root, get_uploads_dir, ) +from ...core.artifact_validation.defaults import default_registry +from ...core.artifact_validation.service import validate_artifact from ...core.execution_scope import resolve_execution_scope from ...core.file_storage import get_user_file_storage, normalize_storage_key from ...core.tools.adapters.vibe.file_tool import read_file @@ -233,7 +236,9 @@ async def _inline_preview_response( media_type: str, file_id: Optional[str] = None, extra_headers: Optional[Dict[str, str]] = None, -) -> FileResponse: + validation_only: bool = False, + public_validation: bool = False, +) -> Response: """Build the final inline preview FileResponse, rasterizing SVG to PNG. Raw SVG bytes are never served inline — an embedded ``