diff --git a/.changeset/shiny-seals-make.md b/.changeset/shiny-seals-make.md new file mode 100644 index 000000000..8edb9a52a --- /dev/null +++ b/.changeset/shiny-seals-make.md @@ -0,0 +1,8 @@ +--- +"@emdash-cms/admin": patch +"emdash": patch +--- + +Fixes the `file` field type rendering as a plain text input in the content editor. Adds a `FileFieldRenderer` that opens the media picker (with mime filter disabled) so any file type can be attached. Also adds a `hideUrlInput` prop to `MediaPickerModal` so non-image pickers can hide the image-specific "Insert from URL" input. + +Aligns the Zod schema and generated TypeScript types for `image` and `file` fields with the shape the admin actually stores: `provider?`, `meta?` (for both), and `previewUrl?` (for image). Previously these fields were stripped on validation and missing from generated types, so site code could not reliably resolve local media URLs from `meta.storageKey`. diff --git a/packages/admin/src/components/ContentEditor.tsx b/packages/admin/src/components/ContentEditor.tsx index 275999417..ff67028cf 100644 --- a/packages/admin/src/components/ContentEditor.tsx +++ b/packages/admin/src/components/ContentEditor.tsx @@ -17,6 +17,7 @@ import { Eye, Image as ImageIcon, MagnifyingGlass, + Paperclip, X, Trash, ArrowsInSimple, @@ -37,8 +38,9 @@ import type { } from "../lib/api"; import { getPreviewUrl, getDraftStatus } from "../lib/api"; import { fromDatetimeLocalInputValue, toDatetimeLocalInputValue } from "../lib/datetime-local.js"; +import { formatFileSize, getFileIcon } from "../lib/media-utils"; import { usePluginAdmins } from "../lib/plugin-context.js"; -import { contentUrl } from "../lib/url.js"; +import { contentUrl, isSafeUrl } from "../lib/url.js"; import { cn, slugify } from "../lib/utils"; import { ArrowPrev } from "./ArrowIcons.js"; import { BlockKitFieldWidget } from "./BlockKitFieldWidget.js"; @@ -1342,6 +1344,24 @@ function FieldRenderer({ ); } + case "file": { + // value is either a FileFieldValue object or undefined. + // The file field type was unusable before this PR (rendered as a text input + // that produced raw strings nobody could meaningfully save), so there is no + // "legacy string" data to preserve here. + const fileValue = + value != null && typeof value === "object" ? (value as FileFieldValue) : undefined; + return ( + + ); + } + case "repeater": { const validation = field.validation; const subFields = (validation?.subFields ?? []) as Array<{ @@ -1666,6 +1686,168 @@ function ImageFieldRenderer({ ); } +/** + * File field value — matches the "file" shape validated by the Zod generator: + * { id, provider?, src?, filename?, mimeType?, size?, meta? } + */ +interface FileFieldValue { + id: string; + /** Provider ID (e.g., "local", "s3") */ + provider?: string; + /** Direct URL for non-local media */ + src?: string; + filename?: string; + mimeType?: string; + size?: number; + /** Provider-specific metadata */ + meta?: Record; +} + +interface FileFieldRendererProps { + id?: string; + label: string; + value: FileFieldValue | undefined; + onChange: (value: FileFieldValue | null) => void; + required?: boolean; +} + +/** + * File field with media picker + * + * Like ImageFieldRenderer but for arbitrary file types. Shows a mime-type-appropriate + * icon, filename, and size instead of an image preview. + */ +function FileFieldRenderer({ id, label, value, onChange, required }: FileFieldRendererProps) { + const { t } = useLingui(); + const [pickerOpen, setPickerOpen] = React.useState(false); + + // Normalize value to derive display info. + // For local files, prefer meta.storageKey; fall back to value.src when it's an + // internal media path; finally fall back to value.id so local files remain + // clickable even when metadata is sparse. For external providers, use value.src + // but only when it's an http(s) URL — a hostile provider plugin could otherwise + // return a data: or javascript: URL that gets rendered as a clickable link. + const normalized = React.useMemo(() => { + if (!value) return null; + const isLocal = !value.provider || value.provider === "local"; + const storageKey = + typeof value.meta?.storageKey === "string" ? value.meta.storageKey : undefined; + const localSrc = + typeof value.src === "string" && value.src.startsWith("/_emdash/") ? value.src : undefined; + // Storage keys come from server-controlled paths today, but the Zod schema + // now lets clients write arbitrary `meta.storageKey` strings via the content + // API. Encode before interpolating so attacker-shaped values can't escape + // the path with `?` or `#`. + const localUrl = isLocal + ? storageKey + ? `/_emdash/api/media/file/${encodeURIComponent(storageKey)}` + : (localSrc ?? `/_emdash/api/media/file/${encodeURIComponent(value.id)}`) + : undefined; + const externalUrl = !isLocal && value.src && isSafeUrl(value.src) ? value.src : undefined; + return { + displayUrl: localUrl ?? externalUrl, + filename: value.filename || t`Untitled file`, + mimeType: value.mimeType || "", + size: value.size, + }; + }, [value, t]); + + const handleSelect = (item: MediaItem) => { + const isLocalProvider = !item.provider || item.provider === "local"; + onChange({ + id: item.id, + provider: item.provider || "local", + src: isLocalProvider ? undefined : item.url, + filename: item.filename, + mimeType: item.mimeType, + size: item.size, + meta: isLocalProvider ? { ...item.meta, storageKey: item.storageKey } : item.meta, + }); + }; + + const handleRemove = () => { + onChange(null); + }; + + const hasMime = !!normalized?.mimeType; + const size = typeof normalized?.size === "number" ? normalized.size : undefined; + const hasSize = size !== undefined; + + return ( +
+ + {normalized ? ( +
+ +
+ {normalized.displayUrl ? ( + + {normalized.filename} + + ) : ( +

{normalized.filename}

+ )} + {(hasMime || hasSize) && ( +

+ {hasMime ? normalized.mimeType : null} + {hasMime && hasSize ? " • " : null} + {hasSize ? formatFileSize(size) : null} +

+ )} +
+
+ + +
+
+ ) : ( + + )} + + {required && !normalized && ( +

{t`This field is required`}

+ )} +
+ ); +} + /** * Author selector component for editors and above */ diff --git a/packages/admin/src/components/MediaPickerModal.tsx b/packages/admin/src/components/MediaPickerModal.tsx index 0a808b6df..47de9489a 100644 --- a/packages/admin/src/components/MediaPickerModal.tsx +++ b/packages/admin/src/components/MediaPickerModal.tsx @@ -9,7 +9,7 @@ import { Button, Dialog, Input, Label, Loader } from "@cloudflare/kumo"; import { plural } from "@lingui/core/macro"; import { useLingui } from "@lingui/react/macro"; -import { Upload, Image, Check, Globe, MagnifyingGlass } from "@phosphor-icons/react"; +import { Upload, Image, Check, Globe, MagnifyingGlass, Paperclip } from "@phosphor-icons/react"; import { X } from "@phosphor-icons/react"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import * as React from "react"; @@ -42,6 +42,18 @@ export interface MediaPickerModalProps { /** Filter by mime type prefix, e.g. "image/" */ mimeTypeFilter?: string; title?: string; + /** + * Hide the "Insert from URL" input. Defaults to false. + * The URL input probes image dimensions and is only meaningful for image pickers, + * so non-image pickers (e.g. generic file pickers) should hide it. + */ + hideUrlInput?: boolean; + /** + * What kind of media this picker is for. Drives user-facing copy + * (default title, empty-state message, upload button label, empty-state icon). + * Defaults to "image" — set to "file" for generic file pickers. + */ + mediaKind?: "image" | "file"; } /** @@ -66,9 +78,17 @@ export function MediaPickerModal({ onSelect, mimeTypeFilter = "image/", title: providedTitle, + hideUrlInput = false, + mediaKind = "image", }: MediaPickerModalProps) { const { t } = useLingui(); - const title = providedTitle ?? t`Select Image`; + const isFileKind = mediaKind === "file"; + const title = providedTitle ?? (isFileKind ? t`Select File` : t`Select Image`); + const emptyStateUploadHint = isFileKind + ? t`Upload a file to get started` + : t`Upload an image to get started`; + const emptyStateUploadCta = isFileKind ? t`Upload File` : t`Upload Image`; + const EmptyStateIcon = isFileKind ? Paperclip : Image; const queryClient = useQueryClient(); const [selectedItem, setSelectedItem] = React.useState(null); const [activeProvider, setActiveProvider] = React.useState("local"); @@ -362,41 +382,45 @@ export function MediaPickerModal({ /> - {/* URL Input */} -
- -
-
- - { - setImageUrl(e.target.value); - setUrlError(null); - }} - onKeyDown={handleUrlKeyDown} - className="ps-9" - /> + {/* URL Input (image pickers only — probes image dimensions) */} + {!hideUrlInput && ( + <> +
+ +
+
+ + { + setImageUrl(e.target.value); + setUrlError(null); + }} + onKeyDown={handleUrlKeyDown} + className="ps-9" + /> +
+ +
+ {urlError &&

{urlError}

}
- -
- {urlError &&

{urlError}

} -
- {/* Divider with "or" */} -
-
- -
-
- {t`or choose from library`} -
-
+ {/* Divider with "or" */} +
+
+ +
+
+ {t`or choose from library`} +
+
+ + )} {/* Provider Tabs */} {providerTabs.length > 1 && ( @@ -487,13 +511,13 @@ export function MediaPickerModal({
) : items.length === 0 ? (
- +
diff --git a/packages/admin/tests/components/ContentEditor.test.tsx b/packages/admin/tests/components/ContentEditor.test.tsx index 6b9416617..5fbaf099b 100644 --- a/packages/admin/tests/components/ContentEditor.test.tsx +++ b/packages/admin/tests/components/ContentEditor.test.tsx @@ -304,6 +304,247 @@ describe("ContentEditor", () => { await expect.element(all[2]!).toBeChecked(); }); + it("renders file fields with a Select file button (not a plain text input)", async () => { + // Regression test for #718: the "file" field kind used to fall through to the + // default case and render a text input, making it impossible to actually attach + // a file. It must render a media picker trigger instead. + const screen = await renderEditor({ + fields: { attachment: { kind: "file", label: "Attachment" } }, + isNew: true, + }); + + // The button that opens the picker should be present and labeled with the + // field's label (accessibility). + const selectBtn = screen.getByRole("button", { name: /Select Attachment/i }); + await expect.element(selectBtn).toBeInTheDocument(); + + // And there must not be a text input inside the file field region — the old + // bug rendered an `` labeled "Attachment" as a plain text field. + // Use the field id (`field-attachment`) as an unconditional positive selector. + const fieldRoot = document.getElementById("field-attachment"); + expect(fieldRoot).not.toBeNull(); + const textInputs = fieldRoot!.querySelectorAll( + 'input:not([type="file"]):not([type="hidden"])', + ); + expect(textInputs).toHaveLength(0); + }); + + it("renders existing file field values as a filename, not a text input", async () => { + const item = makeItem({ + data: { + title: "Test", + body: "", + attachment: { + id: "file-1", + filename: "report.pdf", + mimeType: "application/pdf", + size: 102400, + }, + }, + }); + const screen = await renderEditor({ + isNew: false, + item, + fields: { + title: { kind: "string", label: "Title", required: true }, + attachment: { kind: "file", label: "Attachment" }, + }, + }); + + // Filename should be visible + await expect.element(screen.getByText("report.pdf")).toBeInTheDocument(); + // Change button present (picker is wired up) + await expect.element(screen.getByRole("button", { name: "Change" })).toBeInTheDocument(); + }); + + it("renders 0-byte file size instead of hiding it", async () => { + // Regression test: a previous truthiness check (`const hasSize = normalized?.size`) + // hid the size label for valid 0-byte files even though `formatFileSize(0)` + // returns "0 B". + const item = makeItem({ + data: { + title: "Test", + body: "", + attachment: { + id: "file-empty", + filename: "empty.txt", + mimeType: "text/plain", + size: 0, + }, + }, + }); + const screen = await renderEditor({ + isNew: false, + item, + fields: { + title: { kind: "string", label: "Title", required: true }, + attachment: { kind: "file", label: "Attachment" }, + }, + }); + + await expect.element(screen.getByText("empty.txt")).toBeInTheDocument(); + // "0 B" must be rendered, not silently hidden + await expect.element(screen.getByText(/0\s*B/)).toBeInTheDocument(); + }); + + it("falls back to value.src and then value.id for local files without meta.storageKey", async () => { + // Regression test: local files without meta.storageKey previously lost their + // download link because the URL was only built from storageKey. + const itemWithSrc = makeItem({ + data: { + title: "Test", + body: "", + attachment: { + id: "file-no-key", + provider: "local", + src: "/_emdash/api/media/file/file-no-key", + filename: "backup.zip", + mimeType: "application/zip", + size: 2048, + }, + }, + }); + const screen1 = await renderEditor({ + isNew: false, + item: itemWithSrc, + fields: { + title: { kind: "string", label: "Title", required: true }, + attachment: { kind: "file", label: "Attachment" }, + }, + }); + const link1 = screen1.getByRole("link", { name: "backup.zip" }); + await expect.element(link1).toHaveAttribute("href", "/_emdash/api/media/file/file-no-key"); + + // When src is also missing, fall back to value.id + const itemNoSrc = makeItem({ + data: { + title: "Test", + body: "", + attachment: { + id: "file-fallback", + provider: "local", + filename: "notes.txt", + mimeType: "text/plain", + size: 512, + }, + }, + }); + const screen2 = await renderEditor({ + isNew: false, + item: itemNoSrc, + fields: { + title: { kind: "string", label: "Title", required: true }, + attachment: { kind: "file", label: "Attachment" }, + }, + }); + const link2 = screen2.getByRole("link", { name: "notes.txt" }); + await expect.element(link2).toHaveAttribute("href", "/_emdash/api/media/file/file-fallback"); + }); + + it("does not render data: or javascript: URLs from external providers as links", async () => { + // A hostile external provider plugin could return src: "javascript:..." or + // "data:..."; the file field must not surface either as a clickable . + // Filename should still display as plain text so the user can see what's set. + const item = makeItem({ + data: { + title: "Test", + body: "", + attachment: { + id: "evil-1", + provider: "evil", + src: "javascript:alert(1)", + filename: "ok.txt", + mimeType: "text/plain", + }, + }, + }); + const screen = await renderEditor({ + isNew: false, + item, + fields: { + title: { kind: "string", label: "Title", required: true }, + attachment: { kind: "file", label: "Attachment" }, + }, + }); + + // Filename renders… + await expect.element(screen.getByText("ok.txt")).toBeInTheDocument(); + // …but never as a link with the hostile href. + const fieldRoot = document.getElementById("field-attachment"); + expect(fieldRoot).not.toBeNull(); + expect(fieldRoot!.querySelector('a[href^="javascript:"]')).toBeNull(); + expect(fieldRoot!.querySelector('a[href^="data:"]')).toBeNull(); + }); + + it("encodes path-unsafe characters in storageKey when building the local URL", async () => { + // Server-generated storage keys are flat ULIDs today, but the schema + // now allows clients to write any `meta.storageKey` string via the + // content API. `?` or `#` would otherwise escape the path. + const item = makeItem({ + data: { + title: "Test", + body: "", + attachment: { + id: "x", + provider: "local", + filename: "notes.txt", + mimeType: "text/plain", + meta: { storageKey: "abc?evil#frag" }, + }, + }, + }); + const screen = await renderEditor({ + isNew: false, + item, + fields: { + title: { kind: "string", label: "Title", required: true }, + attachment: { kind: "file", label: "Attachment" }, + }, + }); + const link = screen.getByRole("link", { name: "notes.txt" }); + await expect + .element(link) + .toHaveAttribute("href", "/_emdash/api/media/file/abc%3Fevil%23frag"); + }); + + it("Remove button clears the file field value", async () => { + const onSave = vi.fn(); + const item = makeItem({ + data: { + title: "Test", + body: "", + attachment: { + id: "file-1", + filename: "report.pdf", + mimeType: "application/pdf", + size: 1024, + }, + }, + }); + const screen = await renderEditor({ + isNew: false, + item, + onSave, + fields: { + title: { kind: "string", label: "Title", required: true }, + attachment: { kind: "file", label: "Attachment" }, + }, + }); + + await screen.getByRole("button", { name: "Remove Attachment" }).click(); + // The Select empty-state button replaces the filled state. + await expect + .element(screen.getByRole("button", { name: "Select Attachment" })) + .toBeInTheDocument(); + + await screen.getByRole("button", { name: "Save" }).click(); + expect(onSave).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ attachment: null }), + }), + ); + }); + it("renders datetime fields as datetime-local inputs", async () => { const screen = await renderEditor({ fields: { recall_date: { kind: "datetime", label: "Recall date" } }, diff --git a/packages/admin/tests/components/MediaPickerModal.test.tsx b/packages/admin/tests/components/MediaPickerModal.test.tsx index abc30a593..1ad5912ac 100644 --- a/packages/admin/tests/components/MediaPickerModal.test.tsx +++ b/packages/admin/tests/components/MediaPickerModal.test.tsx @@ -225,6 +225,49 @@ describe("MediaPickerModal", () => { { timeout: 3000 }, ); }); + + it("hideUrlInput hides the URL input section (for non-image pickers)", async () => { + const screen = await renderModal({ hideUrlInput: true }); + + // "Insert from URL" label should not appear when hidden + await expect.element(screen.getByText("Select Image")).toBeInTheDocument(); + expect(document.body.textContent).not.toContain("Insert from URL"); + expect(document.body.textContent).not.toContain("or choose from library"); + + // The URL input itself should not be in the DOM + const urlInput = document.querySelector('input[aria-label="Image URL"]'); + expect(urlInput).toBeNull(); + }); + }); + + describe("mediaKind", () => { + it("uses file-specific copy when mediaKind is 'file'", async () => { + // Use an empty media list so the empty state copy renders. + const api = await import("../../src/lib/api"); + (api.fetchMediaList as any).mockResolvedValueOnce({ items: [] }); + + const screen = await renderModal({ mediaKind: "file", hideUrlInput: true }); + + // Default title should be "Select File", not "Select Image" + await expect.element(screen.getByText("Select File")).toBeInTheDocument(); + expect(document.body.textContent).not.toContain("Select Image"); + + // Empty-state hint and CTA should reference files, not images + await expect.element(screen.getByText("Upload a file to get started")).toBeInTheDocument(); + await expect.element(screen.getByText("Upload File")).toBeInTheDocument(); + expect(document.body.textContent).not.toContain("Upload an image to get started"); + expect(document.body.textContent).not.toContain("Upload Image"); + }); + + it("defaults to image-specific copy when mediaKind is unset", async () => { + const api = await import("../../src/lib/api"); + (api.fetchMediaList as any).mockResolvedValueOnce({ items: [] }); + + const screen = await renderModal(); + + await expect.element(screen.getByText("Select Image")).toBeInTheDocument(); + await expect.element(screen.getByText("Upload an image to get started")).toBeInTheDocument(); + }); }); describe("cancel and close", () => { diff --git a/packages/core/src/schema/zod-generator.ts b/packages/core/src/schema/zod-generator.ts index 7b24414e9..bf7842a67 100644 --- a/packages/core/src/schema/zod-generator.ts +++ b/packages/core/src/schema/zod-generator.ts @@ -131,6 +131,12 @@ function getBaseSchema(type: FieldType, field: Field): ZodTypeAny { alt: z.string().optional(), width: z.number().optional(), height: z.number().optional(), + /** Provider ID (e.g. "local", "cloudflare-images") */ + provider: z.string().optional(), + /** Admin-side preview URL for external providers (not persisted by plugins) */ + previewUrl: z.string().optional(), + /** Provider-specific metadata; for local media this carries storageKey */ + meta: z.record(z.string(), z.unknown()).optional(), }); case "file": @@ -140,6 +146,10 @@ function getBaseSchema(type: FieldType, field: Field): ZodTypeAny { filename: z.string().optional(), mimeType: z.string().optional(), size: z.number().optional(), + /** Provider ID (e.g. "local", "s3") */ + provider: z.string().optional(), + /** Provider-specific metadata; for local media this carries storageKey */ + meta: z.record(z.string(), z.unknown()).optional(), }); case "reference": @@ -384,10 +394,10 @@ function fieldTypeToTypeScript(field: Field): string { return "PortableTextBlock[]"; case "image": - return "{ id: string; src?: string; alt?: string; width?: number; height?: number }"; + return "{ id: string; src?: string; alt?: string; width?: number; height?: number; provider?: string; previewUrl?: string; meta?: Record }"; case "file": - return "{ id: string; src?: string; filename?: string; mimeType?: string; size?: number }"; + return "{ id: string; src?: string; filename?: string; mimeType?: string; size?: number; provider?: string; meta?: Record }"; case "reference": // Could be enhanced to include the referenced collection type