From 3b10cc6feb6365402cb6c92181851ce7ea276c60 Mon Sep 17 00:00:00 2001 From: Geert Theys Date: Fri, 4 Sep 2026 14:20:09 +0700 Subject: [PATCH] assets: browser downloads + loud 404s for file-like paths (#716) Server: - GET /api/assets/raw accepts ?download=1, serving the asset with an attachment Content-Disposition (RFC 5987 filename*) so plain browsers and links save the file under its own name. - The SPA fallback now returns a real 404 for unknown file-looking paths (anything with a non-.html extension, e.g. /files/assets/img.png) instead of index.html with HTTP 200, which made naive download attempts 'succeed' while yielding an HTML file. Unknown app routes still fall through to the SPA shell. Web + Desktop UI (shared app-core Assets view): - New 'Download' context-menu item and a keyboard 'd' on focused asset rows. The shared downloadAsset() helper resolves the asset URL exactly like embedded images do (same-origin HTTP on web, zen-asset:// scheme in the desktop app for local and remote vaults) and saves it via a blob + click, with no bridge-contract change. Tests: Go coverage for the download flag and the fallback 404; vitest coverage for the download helper (resolve failure, read failure, and anchor naming). --- .../httpserver/download_asset_test.go | 114 ++++++++++++++++++ apps/server/internal/httpserver/server.go | 18 ++- .../app-core/src/components/AssetsView.tsx | 14 +++ .../app-core/src/lib/download-asset.test.ts | 66 ++++++++++ packages/app-core/src/lib/download-asset.ts | 27 +++++ 5 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 apps/server/internal/httpserver/download_asset_test.go create mode 100644 packages/app-core/src/lib/download-asset.test.ts create mode 100644 packages/app-core/src/lib/download-asset.ts diff --git a/apps/server/internal/httpserver/download_asset_test.go b/apps/server/internal/httpserver/download_asset_test.go new file mode 100644 index 00000000..8dea8940 --- /dev/null +++ b/apps/server/internal/httpserver/download_asset_test.go @@ -0,0 +1,114 @@ +package httpserver + +import ( + "io/fs" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "testing/fstest" + + "github.com/ZenNotes/zennotes/apps/server/internal/config" + "github.com/ZenNotes/zennotes/apps/server/internal/vault" +) + +// TestAssetDownloadDisposition covers the `?download=1` flag on +// /api/assets/raw (#716): the asset is served with an attachment +// Content-Disposition naming the original file, so plain browsers can save it. +func TestAssetDownloadDisposition(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "assets"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "assets", "pic.png"), []byte("PNGDATA"), 0o600); err != nil { + t.Fatal(err) + } + server, _ := newTestServer(t, config.Config{ + VaultPath: root, + DefaultVaultPath: root, + Bind: "127.0.0.1:7878", + AuthToken: "secret-token", + }) + jar := loginAndJar(t, server, "secret-token") + client := &http.Client{Jar: jar} + + resp, err := client.Get(server.URL + "/api/assets/raw?path=assets/pic.png&download=1") + if err != nil { + t.Fatalf("GET raw?download=1: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", resp.StatusCode) + } + disp := resp.Header.Get("Content-Disposition") + if disp != "attachment; filename*=UTF-8''pic.png" { + t.Fatalf("Content-Disposition = %q", disp) + } + if ct := resp.Header.Get("Content-Type"); ct != "image/png" { + t.Fatalf("Content-Type = %q, want image/png", ct) + } + + // Without the flag, the embed-serving behavior is unchanged: inline, no + // attachment header. + respInline, err := client.Get(server.URL + "/api/assets/raw?path=assets/pic.png") + if err != nil { + t.Fatalf("GET raw: %v", err) + } + defer respInline.Body.Close() + if respInline.StatusCode != http.StatusOK { + t.Fatalf("inline status = %d, want 200", respInline.StatusCode) + } + if disp := respInline.Header.Get("Content-Disposition"); strings.Contains(disp, "attachment") { + t.Fatalf("inline Content-Disposition = %q, want no attachment", disp) + } +} + +// TestStaticFallback404sAssetLikePaths pins the SPA-fallback guard (#716): +// unknown file-looking paths get a real 404 instead of index.html with HTTP +// 200, while unknown app routes still fall through to the SPA shell. +func TestStaticFallback404sAssetLikePaths(t *testing.T) { + static := fstest.MapFS{ + "index.html": &fstest.MapFile{ + Data: []byte("ZenNotes"), + }, + } + root := t.TempDir() + cfg := config.Config{ + VaultPath: root, + DefaultVaultPath: root, + Bind: "127.0.0.1:7878", + AllowInsecureNoAuth: true, + } + v, err := vault.New(cfg.VaultPath, vault.Options{}) + if err != nil { + t.Fatalf("vault.New: %v", err) + } + server := httptest.NewServer(New(v, nil, fs.FS(static), cfg).Router()) + t.Cleanup(server.Close) + + for _, path := range []string{"/files/assets/image.png", "/assets/missing.png", "/notes/export.pdf"} { + resp, err := http.Get(server.URL + path) + if err != nil { + t.Fatalf("GET %s: %v", path, err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusNotFound { + t.Fatalf("GET %s status = %d, want 404", path, resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); strings.Contains(ct, "text/html") { + t.Fatalf("GET %s Content-Type = %q, want non-HTML", path, ct) + } + } + + // Unknown extension-less app routes still hit the SPA fallback. + resp, err := http.Get(server.URL + "/some/app/route") + if err != nil { + t.Fatalf("GET app route: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("app route status = %d, want 200", resp.StatusCode) + } +} diff --git a/apps/server/internal/httpserver/server.go b/apps/server/internal/httpserver/server.go index 0fc1d558..60bf3d3a 100644 --- a/apps/server/internal/httpserver/server.go +++ b/apps/server/internal/httpserver/server.go @@ -3,10 +3,12 @@ package httpserver import ( "encoding/json" "errors" + "fmt" "io/fs" "log" "mime" "net/http" + "net/url" "os" "path/filepath" "runtime" @@ -1026,6 +1028,13 @@ func (s *Server) rawAsset(w http.ResponseWriter, r *http.Request) { if t := mime.TypeByExtension(ext); t != "" { w.Header().Set("Content-Type", t) } + // `?download=1` asks the browser to save the asset instead of rendering + // it (#716). The web Assets view uses this for its Download action, and it + // lets any client turn a plain GET into a file download. + if r.URL.Query().Get("download") == "1" { + w.Header().Set("Content-Disposition", + fmt.Sprintf("attachment; filename*=UTF-8''%s", url.PathEscape(filepath.Base(abs)))) + } w.Header().Set("Cache-Control", "private, max-age=3600") http.ServeFile(w, r, abs) } @@ -1245,7 +1254,14 @@ func (s *Server) serveStatic(w http.ResponseWriter, r *http.Request) { } f, err := s.Static.Open(urlPath) if err != nil { - // SPA fallback: serve index.html for unknown paths. + // SPA fallback: serve index.html for unknown *app* paths. Paths that + // look like files (e.g. /files/assets/image.png) get a real 404, so a + // mistyped asset URL fails loudly instead of "downloading" an HTML + // file with HTTP 200. (#716) + if ext := strings.ToLower(filepath.Ext(urlPath)); ext != "" && ext != ".html" { + http.NotFound(w, r) + return + } s.serveIndexHTML(w) return } diff --git a/packages/app-core/src/components/AssetsView.tsx b/packages/app-core/src/components/AssetsView.tsx index 55e92d8b..e7c6a1c4 100644 --- a/packages/app-core/src/components/AssetsView.tsx +++ b/packages/app-core/src/components/AssetsView.tsx @@ -6,6 +6,7 @@ import { confirmMoveToTrash } from '../lib/confirm-trash' import { promptApp } from '../lib/prompt-requests' import { naturalCompare } from '../lib/natural-sort' import { resolveAssetVaultRelativePath } from '../lib/local-assets' +import { downloadAsset } from '../lib/download-asset' import { ContextMenu, type ContextMenuItem } from './ContextMenu' import { DocumentIcon, ImageIcon, PaperclipIcon, SearchIcon, TrashIcon } from './icons' @@ -166,6 +167,12 @@ export function AssetsView(): JSX.Element { void navigator.clipboard?.writeText(`![[${asset.name}]]`) } + const triggerDownload = (asset: AssetMeta): void => { + downloadAsset(vaultRoot, asset.path).catch((err: unknown) => { + window.alert(err instanceof Error ? err.message : String(err)) + }) + } + const renameAsset = async (asset: AssetMeta): Promise => { if (typeof window.zen.renameAsset !== 'function') return const ext = asset.name.includes('.') ? asset.name.slice(asset.name.lastIndexOf('.')) : '' @@ -190,6 +197,7 @@ export function AssetsView(): JSX.Element { const menuItems = (asset: AssetMeta): ContextMenuItem[] => [ { label: 'Open', onSelect: () => void openNoteInTab(assetTabPath(asset.path)) }, { label: 'Copy embed', onSelect: () => copyEmbed(asset) }, + { label: 'Download', onSelect: () => void triggerDownload(asset) }, { label: 'Reveal in file manager', onSelect: () => void window.zen.revealNote(asset.path) }, { label: 'Rename…', onSelect: () => void renameAsset(asset) }, { @@ -273,6 +281,12 @@ export function AssetsView(): JSX.Element { e.preventDefault() open() } + // Keyboard-first download (#716): `d` on a focused + // row saves the asset, matching the context menu. + if (e.key === 'd' && !e.metaKey && !e.ctrlKey && !e.altKey) { + e.preventDefault() + triggerDownload(asset) + } }} onContextMenu={(e) => { e.preventDefault() diff --git a/packages/app-core/src/lib/download-asset.test.ts b/packages/app-core/src/lib/download-asset.test.ts new file mode 100644 index 00000000..28d868d0 --- /dev/null +++ b/packages/app-core/src/lib/download-asset.test.ts @@ -0,0 +1,66 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { downloadAsset } from './download-asset' + +// The helper talks to the DOM (`window.zen`, `document`, `URL.createObjectURL`) +// and to `fetch`. Vitest here runs in a node environment, so each test stubs +// exactly the surface the helper touches: resolve the embed URL, fetch it, +// and click a hidden anchor carrying the asset's basename. +describe('downloadAsset', () => { + afterEach(() => { + vi.unstubAllGlobals() + delete (URL as unknown as Record).createObjectURL + delete (URL as unknown as Record).revokeObjectURL + }) + + function stubDom(options: { + resolve?: string | null + fetchOk?: boolean + fetchedUrl?: (url: string) => void + }): { anchors: Array<{ download: string; click: ReturnType }> } { + const anchors: Array<{ download: string; click: ReturnType }> = [] + const resolve = 'resolve' in options ? options.resolve : 'https://vault.test/api/assets/raw?path=x' + vi.stubGlobal('window', { + zen: { resolveVaultAssetUrl: vi.fn(() => resolve) } + }) + vi.stubGlobal('document', { + body: { appendChild: vi.fn() }, + createElement: () => { + const anchor = { href: '', download: '', click: vi.fn(), remove: vi.fn() } + anchors.push(anchor) + return anchor + } + }) + ;(URL as unknown as Record).createObjectURL = vi.fn(() => 'blob:mock') + ;(URL as unknown as Record).revokeObjectURL = vi.fn() + vi.stubGlobal( + 'fetch', + vi.fn((url: string | URL) => { + options.fetchedUrl?.(String(url)) + return Promise.resolve({ ok: options.fetchOk ?? true, blob: () => Promise.resolve(new Blob(['PNG'])) }) + }) + ) + return { anchors } + } + + it('fetches the resolved asset URL and clicks an anchor named after the asset', async () => { + let fetched = '' + const { anchors } = stubDom({ fetchedUrl: (url) => (fetched = url) }) + + await downloadAsset('/vault', 'assets/holiday pic.png') + + expect(fetched).toBe('https://vault.test/api/assets/raw?path=x') + expect(anchors).toHaveLength(1) + expect(anchors[0]?.download).toBe('holiday pic.png') + expect(anchors[0]?.click).toHaveBeenCalledOnce() + }) + + it('throws when the bridge cannot resolve the path', async () => { + stubDom({ resolve: null }) + await expect(downloadAsset('/vault', '../escape.png')).rejects.toThrow('Asset path is invalid.') + }) + + it('throws when the asset cannot be read', async () => { + stubDom({ fetchOk: false }) + await expect(downloadAsset('/vault', 'assets/missing.png')).rejects.toThrow('Asset could not be read.') + }) +}) diff --git a/packages/app-core/src/lib/download-asset.ts b/packages/app-core/src/lib/download-asset.ts new file mode 100644 index 00000000..cdc3af6b --- /dev/null +++ b/packages/app-core/src/lib/download-asset.ts @@ -0,0 +1,27 @@ +/** + * Trigger a browser download of a vault asset (#716). + * + * Resolves the asset's URL exactly the way embedded images do — + * same-origin HTTP on web, the `zen-asset://` privileged scheme in the + * desktop app (local or remote vault) — fetches it as a blob, and clicks a + * hidden `` anchor so the browser saves it under the asset's + * own name. Works in the renderer without any bridge-contract change. + */ +export async function downloadAsset( + vaultRoot: string | null, + assetPath: string +): Promise { + const url = window.zen.resolveVaultAssetUrl(vaultRoot ?? '', assetPath) + if (!url) throw new Error('Asset path is invalid.') + const response = await fetch(url) + if (!response.ok) throw new Error('Asset could not be read.') + const blob = await response.blob() + const objectUrl = URL.createObjectURL(blob) + const anchor = document.createElement('a') + anchor.href = objectUrl + anchor.download = assetPath.split('/').pop() ?? assetPath + document.body.appendChild(anchor) + anchor.click() + anchor.remove() + URL.revokeObjectURL(objectUrl) +}