-
Notifications
You must be signed in to change notification settings - Fork 62
feat(mobile): preview generated task artifacts (port #3837) #3867
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
Open
Gilbert09
wants to merge
2
commits into
main
Choose a base branch
from
tom/mobile-preview-task-artifacts
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+482
−9
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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
180 changes: 180 additions & 0 deletions
180
apps/mobile/src/features/tasks/components/ArtifactPreview.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,180 @@ | ||
| import { useQuery } from "@tanstack/react-query"; | ||
| import { ArrowSquareOut, Warning, X } from "phosphor-react-native"; | ||
| import { | ||
| ActivityIndicator, | ||
| Image, | ||
| Modal, | ||
| Pressable, | ||
| ScrollView, | ||
| Text, | ||
| View, | ||
| } from "react-native"; | ||
| import { useSafeAreaInsets } from "react-native-safe-area-context"; | ||
| import WebView from "react-native-webview"; | ||
| import { MarkdownText } from "@/features/chat"; | ||
| import { applyCspToHtml } from "@/features/mcp/sandbox/mcpAppCsp"; | ||
| import { openExternalUrl } from "@/lib/openExternalUrl"; | ||
| import { useThemeColors } from "@/lib/theme"; | ||
| import { useCloudAttachmentPreview } from "../hooks/useCloudAttachmentPreview"; | ||
| import type { TaskRunArtifact } from "../types"; | ||
| import { artifactPreviewKind } from "../utils/artifactPreview"; | ||
|
|
||
| interface ArtifactPreviewProps { | ||
| taskId: string; | ||
| runId: string; | ||
| artifact: TaskRunArtifact; | ||
| onClose: () => void; | ||
| } | ||
|
|
||
| export function ArtifactPreview({ | ||
| taskId, | ||
| runId, | ||
| artifact, | ||
| onClose, | ||
| }: ArtifactPreviewProps) { | ||
| const insets = useSafeAreaInsets(); | ||
| const themeColors = useThemeColors(); | ||
| const name = artifact.name ?? "artifact"; | ||
| const kind = artifactPreviewKind(name); | ||
|
|
||
| const { data: url, isLoading: urlLoading } = useCloudAttachmentPreview( | ||
| taskId, | ||
| artifact.id ? { runId, artifactId: artifact.id } : undefined, | ||
| ); | ||
|
|
||
| // Markdown and HTML render from the file's text; images and the external | ||
| // fallback only need the presigned URL. | ||
| const needsText = kind === "markdown" || kind === "html"; | ||
| const { | ||
| data: text, | ||
| isLoading: textLoading, | ||
| isError: textError, | ||
| } = useQuery({ | ||
| queryKey: ["artifactText", url], | ||
| enabled: needsText && Boolean(url), | ||
| staleTime: Infinity, | ||
| retry: false, | ||
| queryFn: async (): Promise<string> => { | ||
| const response = await fetch(url ?? ""); | ||
| if (!response.ok) throw new Error("Artifact fetch failed"); | ||
| return response.text(); | ||
| }, | ||
| }); | ||
|
|
||
| const loading = urlLoading || (needsText && textLoading); | ||
|
|
||
| return ( | ||
| <Modal | ||
| visible | ||
| animationType="slide" | ||
| presentationStyle="fullScreen" | ||
| onRequestClose={onClose} | ||
| > | ||
| <View className="flex-1 bg-background" style={{ paddingTop: insets.top }}> | ||
| <View className="flex-row items-center gap-3 px-4 pb-2"> | ||
| <Text | ||
| className="flex-1 font-semibold text-[16px] text-gray-12" | ||
| numberOfLines={1} | ||
| > | ||
| {name} | ||
| </Text> | ||
| {url ? ( | ||
| <Pressable | ||
| onPress={() => openExternalUrl(url)} | ||
| hitSlop={8} | ||
| className="active:opacity-60" | ||
| accessibilityLabel="Open externally" | ||
| > | ||
| <ArrowSquareOut size={20} color={themeColors.gray[12]} /> | ||
| </Pressable> | ||
| ) : null} | ||
| <Pressable | ||
| onPress={onClose} | ||
| hitSlop={8} | ||
| className="active:opacity-60" | ||
| accessibilityLabel="Close preview" | ||
| > | ||
| <X size={20} color={themeColors.gray[12]} /> | ||
| </Pressable> | ||
| </View> | ||
|
|
||
| <View className="flex-1" style={{ paddingBottom: insets.bottom }}> | ||
| {loading ? ( | ||
| <View className="flex-1 items-center justify-center"> | ||
| <ActivityIndicator color={themeColors.accent[9]} /> | ||
| </View> | ||
| ) : !url || textError || kind === "unsupported" ? ( | ||
| <Unsupported | ||
| url={url} | ||
| onShare={() => url && openExternalUrl(url)} | ||
| /> | ||
| ) : kind === "image" ? ( | ||
| <Image | ||
| source={{ uri: url }} | ||
| resizeMode="contain" | ||
| style={{ flex: 1, width: "100%" }} | ||
| /> | ||
| ) : kind === "markdown" ? ( | ||
| <ScrollView | ||
| className="flex-1" | ||
| contentContainerStyle={{ padding: 16 }} | ||
| > | ||
| <MarkdownText content={text ?? ""} /> | ||
| </ScrollView> | ||
| ) : ( | ||
| <WebView | ||
| originWhitelist={["*"]} | ||
| source={{ html: applyCspToHtml(text ?? "") }} | ||
| // Untrusted agent output: no scripts, and the sandbox never | ||
| // navigates to a remote page. Only a genuine tap ("click") opens | ||
| // externally — automatic redirects (e.g. a <meta http-equiv= | ||
| // "refresh">) are dropped so previewing a file can't silently | ||
| // launch an attacker URL. The injected CSP blocks remote resource | ||
| // loads on top of that. | ||
| javaScriptEnabled={false} | ||
| setSupportMultipleWindows={false} | ||
| onShouldStartLoadWithRequest={(req) => { | ||
| if ( | ||
| req.url.startsWith("http://") || | ||
| req.url.startsWith("https://") | ||
| ) { | ||
| if (req.navigationType === "click") openExternalUrl(req.url); | ||
| return false; | ||
| } | ||
| return true; | ||
| }} | ||
| style={{ flex: 1, backgroundColor: "#fff" }} | ||
| /> | ||
| )} | ||
| </View> | ||
| </View> | ||
| </Modal> | ||
| ); | ||
| } | ||
|
|
||
| function Unsupported({ | ||
| url, | ||
| onShare, | ||
| }: { | ||
| url: string | null | undefined; | ||
| onShare: () => void; | ||
| }) { | ||
| const themeColors = useThemeColors(); | ||
| return ( | ||
| <View className="flex-1 items-center justify-center gap-4 px-8"> | ||
| <Warning size={28} color={themeColors.gray[9]} /> | ||
| <Text className="text-center text-[14px] text-gray-11"> | ||
| This file can't be previewed here. | ||
| </Text> | ||
| {url ? ( | ||
| <Pressable | ||
| onPress={onShare} | ||
| className="flex-row items-center gap-2 rounded-lg bg-gray-3 px-4 py-2.5 active:opacity-70" | ||
| > | ||
| <ArrowSquareOut size={16} color={themeColors.gray[12]} /> | ||
| <Text className="text-[14px] text-gray-12">Open externally</Text> | ||
| </Pressable> | ||
| ) : null} | ||
| </View> | ||
| ); | ||
| } | ||
65 changes: 65 additions & 0 deletions
65
apps/mobile/src/features/tasks/components/TaskArtifacts.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,65 @@ | ||
| import { createElement } from "react"; | ||
| import { act, create } from "react-test-renderer"; | ||
| import { describe, expect, it, vi } from "vitest"; | ||
| import type { TaskRunArtifact } from "../types"; | ||
| import { TaskArtifacts } from "./TaskArtifacts"; | ||
|
|
||
| const mockUseTaskArtifacts = vi.fn(); | ||
|
|
||
| vi.mock("../hooks/useTaskArtifacts", () => ({ | ||
| useTaskArtifacts: (...args: unknown[]) => mockUseTaskArtifacts(...args), | ||
| })); | ||
|
|
||
| vi.mock("./ArtifactPreview", () => ({ | ||
| ArtifactPreview: (props: Record<string, unknown>) => | ||
| createElement("ArtifactPreview", props), | ||
| })); | ||
|
|
||
| vi.mock("../api", () => ({ presignTaskRunArtifact: vi.fn() })); | ||
|
|
||
| vi.mock("@/lib/openExternalUrl", () => ({ openExternalUrl: vi.fn() })); | ||
|
|
||
| vi.mock("phosphor-react-native", () => ({ | ||
| ArrowSquareOut: (props: Record<string, unknown>) => | ||
| createElement("ArrowSquareOut", props), | ||
| File: (props: Record<string, unknown>) => createElement("File", props), | ||
| })); | ||
|
|
||
| vi.mock("@/lib/theme", () => ({ | ||
| useThemeColors: () => ({ gray: { 9: "#777", 11: "#555" } }), | ||
| })); | ||
|
|
||
| function render(artifacts: TaskRunArtifact[] | undefined) { | ||
| mockUseTaskArtifacts.mockReturnValue({ data: artifacts }); | ||
| let renderer: ReturnType<typeof create> | null = null; | ||
| act(() => { | ||
| renderer = create( | ||
| createElement(TaskArtifacts, { | ||
| taskId: "t1", | ||
| runId: "r1", | ||
| enabled: true, | ||
| }), | ||
| ); | ||
| }); | ||
| if (!renderer) throw new Error("Renderer not created"); | ||
| return JSON.stringify((renderer as ReturnType<typeof create>).toJSON()); | ||
| } | ||
|
|
||
| describe("TaskArtifacts", () => { | ||
| it("renders nothing when there are no artifacts", () => { | ||
| expect(render([])).toBe("null"); | ||
| expect(render(undefined)).toBe("null"); | ||
| }); | ||
|
|
||
| it("lists artifact names and sizes", () => { | ||
| const output = render([ | ||
| { id: "a1", name: "report.md", type: "output", size: 2_400 }, | ||
| { id: "a2", name: "chart.png", type: "output", size: 512 }, | ||
| ]); | ||
| expect(output).toContain("Files"); | ||
| expect(output).toContain("report.md"); | ||
| expect(output).toContain("chart.png"); | ||
| expect(output).toContain("2 KB"); | ||
| expect(output).toContain("512 B"); | ||
| }); | ||
| }); |
107 changes: 107 additions & 0 deletions
107
apps/mobile/src/features/tasks/components/TaskArtifacts.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,107 @@ | ||
| import { ArrowSquareOut, File as FileIcon } from "phosphor-react-native"; | ||
| import { useCallback, useState } from "react"; | ||
| import { ActivityIndicator, Alert, Pressable, Text, View } from "react-native"; | ||
| import { openExternalUrl } from "@/lib/openExternalUrl"; | ||
| import { useThemeColors } from "@/lib/theme"; | ||
| import { presignTaskRunArtifact } from "../api"; | ||
| import { useTaskArtifacts } from "../hooks/useTaskArtifacts"; | ||
| import type { TaskRunArtifact } from "../types"; | ||
| import { formatArtifactSize } from "../utils/artifactPreview"; | ||
| import { ArtifactPreview } from "./ArtifactPreview"; | ||
|
|
||
| interface TaskArtifactsProps { | ||
| taskId: string | undefined; | ||
| runId: string | undefined; | ||
| // Gate the manifest fetch on a terminal run, mirroring desktop. | ||
| enabled: boolean; | ||
| } | ||
|
|
||
| export function TaskArtifacts({ taskId, runId, enabled }: TaskArtifactsProps) { | ||
| const themeColors = useThemeColors(); | ||
| const { data: artifacts } = useTaskArtifacts(taskId, runId, enabled); | ||
| const [preview, setPreview] = useState<TaskRunArtifact | null>(null); | ||
| const [sharingId, setSharingId] = useState<string | null>(null); | ||
|
|
||
| const shareArtifact = useCallback( | ||
| async (artifact: TaskRunArtifact): Promise<void> => { | ||
| if (!taskId || !runId || !artifact.storage_path) return; | ||
| setSharingId(artifact.id ?? artifact.storage_path); | ||
| try { | ||
| const url = await presignTaskRunArtifact( | ||
| taskId, | ||
| runId, | ||
| artifact.storage_path, | ||
| ); | ||
| openExternalUrl(url); | ||
| } catch { | ||
| Alert.alert("Couldn't open file", "Please try again."); | ||
| } finally { | ||
| setSharingId(null); | ||
| } | ||
| }, | ||
| [taskId, runId], | ||
| ); | ||
|
|
||
| if (!taskId || !runId || !artifacts || artifacts.length === 0) return null; | ||
|
|
||
| return ( | ||
| <View className="mx-4 mb-2 rounded-lg border border-gray-5 bg-gray-2 p-3"> | ||
| <Text className="mb-2 font-semibold text-[13px] text-gray-12">Files</Text> | ||
| <View className="gap-1"> | ||
| {artifacts.map((artifact) => { | ||
| const sharingKey = artifact.id ?? artifact.storage_path; | ||
| const size = formatArtifactSize(artifact.size); | ||
| const canPreview = Boolean(artifact.id); | ||
| return ( | ||
| <View | ||
| key={sharingKey ?? artifact.name} | ||
| className="flex-row items-center gap-3 rounded-md bg-background px-2 py-1.5" | ||
| > | ||
| <Pressable | ||
| className="min-w-0 flex-1 flex-row items-center gap-2 active:opacity-70" | ||
| disabled={!canPreview} | ||
| onPress={() => setPreview(artifact)} | ||
| > | ||
| <FileIcon size={16} color={themeColors.gray[11]} /> | ||
| <Text | ||
| className="flex-shrink text-[13px] text-gray-12" | ||
| numberOfLines={1} | ||
| > | ||
| {artifact.name ?? "artifact"} | ||
| </Text> | ||
| {size ? ( | ||
| <Text className="text-[12px] text-gray-9">{size}</Text> | ||
| ) : null} | ||
| </Pressable> | ||
| <Pressable | ||
| hitSlop={8} | ||
| className="active:opacity-60" | ||
| disabled={!artifact.storage_path} | ||
| onPress={() => void shareArtifact(artifact)} | ||
| accessibilityLabel="Open externally" | ||
| > | ||
| {sharingId === sharingKey ? ( | ||
| <ActivityIndicator | ||
| size="small" | ||
| color={themeColors.gray[11]} | ||
| /> | ||
| ) : ( | ||
| <ArrowSquareOut size={16} color={themeColors.gray[11]} /> | ||
| )} | ||
| </Pressable> | ||
| </View> | ||
| ); | ||
| })} | ||
| </View> | ||
|
|
||
| {preview?.id ? ( | ||
| <ArtifactPreview | ||
| taskId={taskId} | ||
| runId={runId} | ||
| artifact={preview} | ||
| onClose={() => setPreview(null)} | ||
| /> | ||
| ) : null} | ||
| </View> | ||
| ); | ||
| } |
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.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Low: Markdown previews fetch attacker-controlled images
A generated artifact containing
causesMarkdownTextto renderMarkdownImage, which immediately requests the URL throughImage.getSizeandImage. An attacker can therefore make the device issue HTTP requests to arbitrary Internet or local-network services when the user opens the preview. Disable remote images for artifact previews, require explicit user confirmation, or fetch them through a suitably restricted proxy.