Fix(assets): attached files land in assets/, and uploads are no longer corrupted

src/bridge/imported-assets.test.ts

@@ -0,0 +1,119 @@
+import assert from 'node:assert/strict'
+import { describe, it } from 'node:test'
+import { assetUploadOptions } from './remote-client.ts'
+import {
+  importedAssetFilename,
+  importedAssetMarkdown,
+  importedAssetRelPath
+} from './imported-assets.ts'
+
+describe('importedAssetFilename', () => {
+  it('scrubs characters that would break or retarget a wikilink embed', () => {
+    assert.equal(importedAssetFilename('diagram [v2] #3.png'), 'diagram -v2- -3.png')
+    assert.equal(importedAssetFilename('diagram%5D^draft|wide.png'), 'diagram-5D-draft-wide.png')
+  })
+
+  it('drops path components and control characters before writing a file', () => {
+    assert.equal(importedAssetFilename('../folder/line\nbreak.png'), 'line-break.png')
+    assert.equal(importedAssetFilename('..'), 'file')
+  })
+})
+
+describe('importedAssetRelPath', () => {
+  it('puts an attached file in assets/, never the vault root', () => {
+    // The bug: attach wrote to the vault root, scattering images among the
+    // notes, while paste and the cloud upload both used assets/.
+    assert.equal(importedAssetRelPath('shape_image 18.jpg'), 'assets/shape_image 18.jpg')
+    assert.equal(importedAssetRelPath('report.pdf'), 'assets/report.pdf')
+  })
+})
+
+describe('importedAssetMarkdown', () => {
+  it('links an image the same way a pasted image is linked', () => {
+    const rel = importedAssetRelPath('diagram.png')
+    assert.equal(importedAssetMarkdown(rel, 'diagram.png', 'image'), '![[assets/diagram.png]]')
+  })
+
+  it('never emits a note-relative ../ path', () => {
+    const markdown = importedAssetMarkdown(
+      importedAssetRelPath('shape_image 18.jpg'),
+      'shape_image 18.jpg',
+      'image'
+    )
+    assert.equal(markdown, '![[assets/shape_image 18.jpg]]')
+    assert.ok(!markdown.includes('../'))
+  })
+
+  it('angle-brackets a non-image link so a space cannot break it', () => {
+    const rel = importedAssetRelPath('year end.pdf')
+    assert.equal(importedAssetMarkdown(rel, 'year end.pdf', 'pdf'), '[year end.pdf](<assets/year end.pdf>)')
+  })
+
+  it('escapes a closing angle bracket in the path', () => {
+    assert.equal(
+      importedAssetMarkdown('assets/we>ird.zip', 'we>ird.zip', 'file'),
+      '[we>ird.zip](<assets/we%3Eird.zip>)'
+    )
+  })
+})
+
+describe('assetUploadOptions', () => {
+  const options = assetUploadOptions({
+    url: 'https://notes.example.test/api/assets/upload',
+    fileName: 'diagram.png',
+    base64Data: 'AQID',
+    targetDir: 'assets'
+  })
+
+  it('sends the file as a native base64File part, not as base64 text in the body', () => {
+    // The bug: the body was hand-built with `Content-Transfer-Encoding: base64`,
+    // which multipart/form-data parsers ignore (RFC 7578), so Go wrote the
+    // base64 TEXT to disk as the file's bytes and every attachment was corrupt.
+    assert.equal(options.dataType, 'formData')
+    assert.deepEqual(options.data, [
+      { type: 'string', key: 'dir', value: 'assets' },
+      {
+        type: 'base64File',
+        key: 'file',
+        fileName: 'diagram.png',
+        contentType: 'application/octet-stream',
+        value: 'AQID'
+      }
+    ])
+    assert.equal(typeof options.data, 'object')
+  })
+
+  it('declares the boundary its body will be written with', () => {
+    const contentType = options.headers['Content-Type']
+    assert.match(contentType, /^multipart\/form-data; boundary=----ZenNotesUpload[0-9a-f]+$/)
+  })
+
+  it('sanitizes the filename used in the native multipart header', () => {
+    const quoted = assetUploadOptions({
+      url: 'https://notes.example.test/api/assets/upload',
+      fileName: 'we"ird".png',
+      base64Data: 'AQID'
+    })
+    const file = quoted.data[1] as { fileName: string }
+    assert.equal(file.fileName, 'we-ird-.png')
+  })
+
+  it('strips newlines from the filename so it cannot inject multipart headers', () => {
+    const injected = assetUploadOptions({
+      url: 'https://notes.example.test/api/assets/upload',
+      fileName: 'pic.png"\r\nX-Injected: yes',
+      base64Data: 'AQID'
+    })
+    const file = injected.data[1] as { fileName: string }
+    assert.equal(file.fileName, 'pic.png---X-Injected- yes')
+  })
+
+  it('defaults the target directory to empty rather than sending undefined', () => {
+    const bare = assetUploadOptions({
+      url: 'https://notes.example.test/api/assets/upload',
+      fileName: 'a.png',
+      base64Data: 'AQID'
+    })
+    assert.deepEqual(bare.data[0], { type: 'string', key: 'dir', value: '' })
+  })
+})

src/bridge/imported-assets.ts

@@ -0,0 +1,50 @@
+/**
+ * Where an imported file goes and how it is linked.
+ *
+ * A leaf module on purpose: `vault-core` reaches `@shared/*` through a Vite
+ * alias, which plain `node --test` cannot resolve, so nothing there is
+ * unit-testable. These rules are where the attachment bugs hid, so they live
+ * where a test can reach them. The vault core re-exports them so existing
+ * importers remain unaffected.
+ */
+import type { ImportedAssetKind } from '@bridge-contract/ipc'
+
+export const ASSETS_DIR = 'assets'
+
+/** Keep imported names valid as both filenames and wikilink targets. Pasted
+ * images already apply this rule: brackets, anchors, and pipes can terminate
+ * or retarget a wikilink, while path/control characters are unsafe on one or
+ * more supported filesystems. */
+export function importedAssetFilename(filename: string): string {
+  const segments = filename.split(/[\\/]/)
+  const leaf = segments[segments.length - 1] ?? ''
+  const safe = leaf
+    // eslint-disable-next-line no-control-regex
+    .replace(/[\\/:%\u0000-\u001f*?"<>|[\]#^]/g, '-')
+    .replace(/\s+/g, ' ')
+    .trim()
+  return safe && safe !== '.' && safe !== '..' ? safe : 'file'
+}
+
+/** Every imported file lands in `assets/`, whichever way it arrived. Paste,
+ *  attach and the cloud upload agree on this, so a link written by one is
+ *  resolvable by the others. Attach used to write to the vault root, which
+ *  scattered images among the notes (Discord, xenin); desktop had the same bug
+ *  and fixed it in ZenNotes#377, and the port here was missed. */
+export function importedAssetRelPath(filename: string): string {
+  return `${ASSETS_DIR}/${filename}`
+}
+
+/** The link written into the note, by vault-relative path. Images take the
+ *  wikilink form, matching pasted images and the cloud vault; anything else
+ *  gets an angle-bracketed markdown link so a space in the name cannot break
+ *  it. Never a note-relative `../` path: the old attach path hand-built one
+ *  from the note's depth, which pointed outside the vault from a nested note. */
+export function importedAssetMarkdown(
+  relPath: string,
+  filename: string,
+  kind: ImportedAssetKind
+): string {
+  if (kind === 'image') return `![[${relPath}]]`
+  return `[${filename}](<${relPath.replace(/>/g, '%3E')}>)`
+}

src/bridge/remote-client.ts

@@ -26,6 +26,7 @@ import type {
   VaultTextSearchMatch
 } from '@shared/ipc'
 import type { VaultTask } from '@shared/tasks'
+import { importedAssetFilename } from './imported-assets.ts'
 
 export interface RemoteClientOptions {
   baseUrl: string
@@ -40,6 +41,58 @@ const TIMEOUT_MS = 15000
  * load-bearing for the absence-aware database reader in remote-vault.ts,
  * which must never read "the request never arrived" as "the file is absent".
  */
+export interface AssetUploadRequest {
+  url: string
+  fileName: string
+  base64Data: string
+  targetDir?: string
+}
+
+/**
+ * The multipart upload for an attachment.
+ *
+ * This used to hand-build the body as one string, with the file part carrying
+ * `Content-Transfer-Encoding: base64` and the base64 text as its content. That
+ * header is a MIME construct: RFC 7578 dropped it from multipart/form-data, and
+ * Go's `mime/multipart` (what the server parses with) does not honour it. The
+ * server therefore wrote the base64 TEXT to disk as the file's bytes, so an
+ * attachment arrived in `assets/` with the right name, about a third larger
+ * than the original, and unreadable by anything (zennotesandroid#40).
+ *
+ * The bytes have to stay base64 to cross the native bridge at all, since a
+ * CapacitorHttp string body is written out as UTF-8 and would mangle anything
+ * above 0x7F. `dataType: 'formData'` is the supported way through: both native
+ * layers decode a `base64File` entry and write the raw bytes into the part
+ * (Android `CapacitorHttpUrlConnection.writeFormDataRequestBody`, iOS
+ * `CapacitorUrlRequest.getRequestDataFromFormData`), which is what the server
+ * expects. The sibling direct-object upload already used the same mechanism
+ * with `dataType: 'file'`.
+ */
+export function assetUploadOptions(request: AssetUploadRequest) {
+  // Both native layers reuse the boundary from the header when it carries one,
+  // so the body and the header cannot disagree.
+  const boundary = `----ZenNotesUpload${Date.now().toString(16)}`
+  return {
+    url: request.url,
+    headers: { 'Content-Type': `multipart/form-data; boundary=${boundary}` },
+    dataType: 'formData' as const,
+    data: [
+      { type: 'string', key: 'dir', value: request.targetDir ?? '' },
+      {
+        type: 'base64File',
+        key: 'file',
+        // Capacitor inserts this value directly into Content-Disposition on
+        // both native platforms, so use the same safe name written to disk.
+        fileName: importedAssetFilename(request.fileName),
+        contentType: 'application/octet-stream',
+        value: request.base64Data
+      }
+    ],
+    connectTimeout: TIMEOUT_MS,
+    readTimeout: TIMEOUT_MS
+  }
+}
+
 export class RemoteRequestError extends Error {
   readonly status: number
 
@@ -292,29 +345,18 @@ export class RemoteClient {
     return { base64: response.data, mimeType: contentType }
   }
 
-  /** Multipart upload — CapacitorHttp has no FormData bridge, so send the
-   *  body prebuilt with an explicit boundary. */
   async uploadAsset(fileName: string, base64Data: string, targetDir = ''): Promise<AssetMeta> {
-    const boundary = `----ZenNotesUpload${Date.now().toString(16)}`
-    const head =
-      `--${boundary}\r\n` +
-      `Content-Disposition: form-data; name="dir"\r\n\r\n${targetDir}\r\n` +
-      `--${boundary}\r\n` +
-      `Content-Disposition: form-data; name="file"; filename="${fileName.replace(/"/g, '')}"\r\n` +
-      `Content-Type: application/octet-stream\r\n` +
-      `Content-Transfer-Encoding: base64\r\n\r\n`
-    const tail = `\r\n--${boundary}--\r\n`
+    const options = assetUploadOptions({
+      url: `${this.baseUrl}/api/assets/upload`,
+      fileName,
+      base64Data,
+      targetDir
+    })
     let response: HttpResponse
     try {
       response = await CapacitorHttp.post({
-        url: `${this.baseUrl}/api/assets/upload`,
-        headers: {
-          ...this.headers(),
-          'Content-Type': `multipart/form-data; boundary=${boundary}`
-        },
-        data: head + base64Data + tail,
-        connectTimeout: TIMEOUT_MS,
-        readTimeout: TIMEOUT_MS
+        ...options,
+        headers: { ...this.headers(), ...options.headers }
       })
     } catch (error) {
       throw new Error(connectionErrorMessage(this.baseUrl, error))

src/bridge/remote-vault.ts

@@ -34,6 +34,7 @@ import type { CustomTemplateFile, WriteTemplateInput } from '@bridge-contract/te
 import type { ImportedAsset } from '@shared/ipc'
 import { createAbsenceAwareReader } from '@shared/remote-absence'
 import { emitVaultChange } from './events'
+import { importedAssetFilename } from './imported-assets'
 import { RemoteClient, RemoteRequestError } from './remote-client'
 
 function remoteOnly(what: string): never {
@@ -347,7 +348,11 @@ export class RemoteVault {
 
   async importDroppedFile(_notePath: string, file: File): Promise<ImportedAsset> {
     const bytes = new Uint8Array(await file.arrayBuffer())
-    const meta = await this.client.uploadAsset(file.name, bytesToBase64(bytes), 'assets')
+    const meta = await this.client.uploadAsset(
+      importedAssetFilename(file.name),
+      bytesToBase64(bytes),
+      'assets'
+    )
     const isImage = /\.(png|jpe?g|gif|webp|svg|heic)$/i.test(meta.name)
     emitVaultChange({ kind: 'add', path: meta.path, folder: 'inbox', scope: 'content' })
     return {

src/bridge/vault-core.ts

@@ -12,10 +12,19 @@ import {
   systemFolderForDirName,
   type SystemFolderPaths
 } from '@shared/system-folder-paths'
+import { ASSETS_DIR } from './imported-assets'
+
+// Re-exported so every existing importer keeps reaching them here; they live in
+// a leaf module because this one cannot be loaded by `node --test`.
+export {
+  ASSETS_DIR,
+  importedAssetFilename,
+  importedAssetMarkdown,
+  importedAssetRelPath
+} from './imported-assets'
 
 export const FOLDERS: NoteFolder[] = ['inbox', 'quick', 'archive', 'trash']
 export const SYSTEM_FOLDERS = new Set<string>(FOLDERS)
-export const ASSETS_DIR = 'assets'
 export const PRIMARY_ATTACHMENTS_DIR = 'attachements'
 export const LEGACY_ATTACHMENTS_DIRS = [PRIMARY_ATTACHMENTS_DIR, '_assets']
 export const ATTACHMENTS_DIRS = [ASSETS_DIR, ...LEGACY_ATTACHMENTS_DIRS]

src/bridge/vault-fs.ts

@@ -48,6 +48,9 @@ import {
   firstMatchColumn,
   folderForRelativePath,
   hiddenPrimaryRootNames,
+  importedAssetFilename,
+  importedAssetMarkdown,
+  importedAssetRelPath,
   isExcalidrawPath,
   isMarkdownPath,
   joinPath,
@@ -1228,20 +1231,27 @@ export class MobileVault {
     return { name: filename, path: rel, markdown: `![[${rel}]]`, kind: 'image' }
   }
 
-  async importDroppedFile(notePath: string, file: File): Promise<ImportedAsset> {
-    const filename = await this.uniqueFilename('', file.name)
+  /**
+   * An attached file lands in `assets/`, the same folder a pasted image goes to
+   * (`importPastedImage`) and the same one the cloud vault uploads to, and is
+   * linked by vault-relative path.
+   *
+   * It used to be written to the vault ROOT and linked with a hand-built
+   * `'../'.repeat(depth)` path, so attaching a file scattered images among the
+   * notes and produced `![name](<../name.jpg>)` (reported on Discord by xenin).
+   * Desktop had exactly this bug and fixed it in #377; the port was missed
+   * here. Images take the wikilink form so every surface resolves them the same
+   * way paste already does.
+   */
+  async importDroppedFile(_notePath: string, file: File): Promise<ImportedAsset> {
     const bytes = new Uint8Array(await file.arrayBuffer())
-    await this.fs.writeBase64(filename, bytesToBase64(bytes))
+    await this.fs.mkdir(ASSETS_DIR)
+    const filename = await this.uniqueFilename(ASSETS_DIR, importedAssetFilename(file.name))
+    const rel = importedAssetRelPath(filename)
+    await this.fs.writeBase64(rel, bytesToBase64(bytes))
     const kind = classifyImportedAsset(filename)
-    const noteDir = dirName(resolveSafeRel(notePath))
-    const relFromNote = noteDir
-      ? `${'../'.repeat(noteDir.split('/').length)}${filename}`
-      : filename
-    const dest = `<${relFromNote.replace(/>/g, '%3E')}>`
-    const markdown =
-      kind === 'image' ? `![${stemName(filename)}](${dest})` : `[${filename}](${dest})`
-    emitVaultChange({ kind: 'add', path: filename, folder: 'inbox', scope: 'content' })
-    return { name: filename, path: filename, markdown, kind }
+    emitVaultChange({ kind: 'add', path: rel, folder: 'inbox', scope: 'content' })
+    return { name: filename, path: rel, markdown: importedAssetMarkdown(rel, filename, kind), kind }
   }
 
   async renameAsset(relPath: string, nextName: string): Promise<AssetMeta> {