fix(docsync): switches that did not switch off, a webhook nobody could authenticate, and a counter that only grew

More from the audit, each checked against the code before it was touched.

The webhook obeyed neither the Documents addon nor the app_settings kill
switch, so "switch document sync off" stopped the poll while every provider
holding a webhook carried on driving full runs. Both are now read on arrival
and again when the debounce timer fires, so a switch thrown mid-window still
takes effect.

Nextcloud's subscription registered its secret under `X-TREK-Docsync-Signature`
while the endpoint reads `x-trek-docsync-secret`: the subscription existed, the
calls arrived, and every one of them was dropped for carrying no secret the
controller could find. The two names now agree, and the test pins the one the
endpoint actually reads.

A WebDAV create was an unconditional PUT, so a name already taken upstream was
silently overwritten — two trips bound to one folder, or a `receipt.pdf` a
person put there by hand, is enough. Creates now send `If-None-Match: *`.

The per-item retry bookkeeping was stuck at both ends: the backoff was computed
at step 1 every time, so three of the curve's four steps were unreachable, and
the attempt counter only ever grew, which shelved a row that failed once a
month after six months of otherwise healthy syncing. A successful pass now
clears it, which is what makes the limit mean six failures in a row.

Also: a manual run on an orphaned binding is refused with 409 (its credential
belongs to somebody no longer on the trip, and the scheduler already refuses
it, so the button was a way around that); a document deleted while a run is in
flight is no longer uploaded anyway; a path-shaped WebDAV id containing `..` is
rejected instead of addressing a file outside the bound folder; and Papra's
multipart part header no longer takes an unescaped media type, which a CR could
end early.

Client: three writes in useDocSync swallowed their failure into `void` call
sites, a load failure blanked the panel to "no providers available" whatever
the cause, and overlapping loads let the slowest response win. Writes now
report, only 403/404 blanks the panel, and a generation stamp keeps the newest
answer. The dead lastRun state went with them.

client/src/components/Files/docsync/useDocSync.test.ts

@@ -212,7 +212,7 @@ describe('useDocSync spinner', () => {
     listLinksApi.mockReturnValueOnce(gate.promise)
     seenLoading.length = 0
 
-    let pending!: Promise<void>
+    let pending!: Promise<boolean>
     await act(async () => { pending = result.current.updateLink(1, { syncEnabled: false }) })
     expect(result.current.loading).toBe(false)
 
@@ -235,7 +235,7 @@ describe('useDocSync updateLink', () => {
     const gate = deferred<unknown>()
     updateLinkApi.mockReturnValueOnce(gate.promise)
 
-    let pending!: Promise<void>
+    let pending!: Promise<boolean>
     await act(async () => { pending = result.current.updateLink(1, { direction: 'pull' }) })
 
     expect(result.current.links[0].direction).toBe('pull')
@@ -257,14 +257,18 @@ describe('useDocSync updateLink', () => {
 
   it('FE-DOCSYNC-HOOK-011: a rejected update does not leave the optimistic value standing', async () => {
     const { result } = await mountLoaded()
-    updateLinkApi.mockRejectedValueOnce(new Error('conflict'))
+    updateLinkApi.mockRejectedValueOnce({ response: { data: { error: 'forbidden' } } })
+    listLinksApi.mockResolvedValueOnce([link({ direction: 'both' })])
 
-    await act(async () => {
-      await expect(result.current.updateLink(1, { syncEnabled: false })).rejects.toThrow('conflict')
-    })
+    let ok: boolean | undefined
+    await act(async () => { ok = await result.current.updateLink(1, { direction: 'pull' }) })
 
-    expect(listLinksApi).toHaveBeenCalledTimes(2)
-    expect(result.current.links[0].syncEnabled).toBe(true)
+    // Reported rather than thrown: both shells call this as `void`, so a
+    // rejection reached nobody. The reload puts the server's answer back and
+    // the reason stays on screen.
+    expect(ok).toBe(false)
+    expect(result.current.links[0].direction).toBe('both')
+    expect(result.current.error).toBe('forbidden')
   })
 })
 
@@ -333,19 +337,24 @@ describe('useDocSync run and save feedback', () => {
     await act(async () => { await result.current.syncNow(1) })
 
     expect(syncNowApi).toHaveBeenCalledWith(TRIP, 1, false)
-    expect(result.current.lastRunFor(1)).toEqual({ pulled: 4, pushed: 2 })
-    // a binding nobody ran in this session reports zeros, not someone else numbers
-    expect(result.current.lastRunFor(2)).toEqual({ pulled: 0, pushed: 0 })
+    // What a run moved is no longer kept: the flow bar shows standing holdings,
+    // which is the question somebody opening the dialog actually has. The run's
+    // own numbers were only ever on screen for the moment after pressing it.
+    expect(result.current.links).toHaveLength(1)
   })
 
-  it('FE-DOCSYNC-HOOK-017: a run that reports no numbers counts as nothing moved', async () => {
-    syncNowApi.mockResolvedValue({ state: 'ok' })
+  it('FE-DOCSYNC-HOOK-017: a refused run says so instead of vanishing into a void call', async () => {
+    // syncNow is called as `void sync.syncNow(...)` from both shells, so a
+    // rejection that escaped the hook reached nobody. An orphaned binding
+    // answers 409 here, and the person pressing the button deserves the reason.
+    syncNowApi.mockRejectedValueOnce({ response: { data: { error: 'link_orphaned' } } })
     const { result } = await mountLoaded()
 
-    await act(async () => { await result.current.syncNow(1, true) })
+    let outcome: unknown
+    await act(async () => { outcome = await result.current.syncNow(1) })
 
-    expect(syncNowApi).toHaveBeenCalledWith(TRIP, 1, true)
-    expect(result.current.lastRunFor(1)).toEqual({ pulled: 0, pushed: 0 })
+    expect(outcome).toBeNull()
+    expect(result.current.error).toBe('link_orphaned')
   })
 
   it('FE-DOCSYNC-HOOK-018: a refused save reports failure and surfaces the server reason', async () => {

client/src/components/Files/docsync/useDocSync.ts

@@ -92,12 +92,6 @@ export function useDocSync(tripId: number | string, enabled: boolean) {
   const [connections, setConnections] = useState<DocSyncConnection[]>([])
   const [links, setLinks] = useState<DocSyncLink[]>([])
   const [itemCounts, setItemCounts] = useState<Record<string, number>>({})
-  /**
-   * What the last run of each binding moved, so the flow bar can show numbers
-   * on its lanes. Kept in memory rather than persisted: it describes one run,
-   * and after a reload "nothing moved since you got here" is the honest answer.
-   */
-  const [lastRun, setLastRun] = useState<Record<number, { pulled: number; pushed: number }>>({})
   const [loading, setLoading] = useState(false)
   const [busy, setBusy] = useState<string | null>(null)
   const [error, setError] = useState<string | null>(null)
@@ -111,8 +105,21 @@ export function useDocSync(tripId: number | string, enabled: boolean) {
    */
   const loadedOnce = useRef(false)
 
+  /**
+   * Which load is the current one.
+   *
+   * Four requests go out per load and several loads overlap — every mutation
+   * starts one, and so does every ping. Without a generation stamp the slowest
+   * response wins, which on a binding somebody just changed means the screen
+   * settles on the state from before the change.
+   */
+  const generation = useRef(0)
+  /** Whether the last failure came from loading rather than from a write. */
+  const loadFailed = useRef(false)
+
   const load = useCallback(async () => {
     if (!enabled) return
+    const mine = ++generation.current
     if (!loadedOnce.current) setLoading(true)
     try {
       const [p, c, l, s] = await Promise.all([
@@ -121,6 +128,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) {
         docsyncApi.listLinks(tripId),
         docsyncApi.status(tripId),
       ])
+      if (mine !== generation.current) return
       setProviders(p as DocSyncProvider[])
       setConnections(c as DocSyncConnection[])
       // The status route carries the holdings; the links route does not, so the
@@ -129,14 +137,28 @@ export function useDocSync(tripId: number | string, enabled: boolean) {
       const holdingsById = new Map((status.links ?? []).map(x => [x.id, x.holdings]))
       setLinks((l as DocSyncLink[]).map(x => ({ ...x, holdings: holdingsById.get(x.id) })))
       setItemCounts(status.items || {})
-      setError(null)
+      // Only a load failure is cleared here. A write that just failed reloads to
+      // put the server's answer back on screen, and clearing unconditionally
+      // wiped the reason it had set a moment earlier.
+      setError(prev => (prev !== null && loadFailed.current ? null : prev))
+      loadFailed.current = false
       loadedOnce.current = true
-    } catch {
-      // A failure here means the addon is off or the user lost access, both of
-      // which the panel renders as "nothing to configure" rather than an alarm.
-      setProviders([])
+    } catch (e: unknown) {
+      // A 403 or 404 really does mean "nothing to configure here" — the addon is
+      // off or this person lost access — and blanking the panel is the honest
+      // answer. Anything else is a failure to say so: blanking on a dropped
+      // connection told the user their provider list was empty.
+      if (mine !== generation.current) return
+      const status = (e as { response?: { status?: number } })?.response?.status
+      loadFailed.current = true
+      if (status === 403 || status === 404) {
+        setProviders([])
+        setError(null)
+      } else {
+        setError(readError(e))
+      }
     } finally {
-      setLoading(false)
+      if (mine === generation.current) setLoading(false)
     }
   }, [tripId, enabled])
 
@@ -228,20 +250,40 @@ export function useDocSync(tripId: number | string, enabled: boolean) {
     }
   }, [tripId, load])
 
+  /**
+   * Change one binding, optimistically.
+   *
+   * The reload in `finally` puts the server's answer back either way, so a
+   * refused change corrects itself on screen — but it used to do that in
+   * silence, with the rejection escaping into a `void` call site. The switch
+   * simply flipped back and nobody said why.
+   */
   const updateLink = useCallback(async (linkId: number, patch: Record<string, unknown>) => {
     setLinks(prev => prev.map(l => (l.id === linkId ? { ...l, ...patch } as DocSyncLink : l)))
+    setError(null)
     try {
       await docsyncApi.updateLink(tripId, linkId, patch)
-    } finally {
       await load()
+      return true
+    } catch (e: unknown) {
+      // Reload first, so the optimistic value is corrected, then say why —
+      // the other way round the reload cleared the message again.
+      await load()
+      setError(readError(e))
+      return false
     }
   }, [tripId, load])
 
   const removeLink = useCallback(async (linkId: number) => {
     setBusy('link')
+    setError(null)
     try {
       await docsyncApi.deleteLink(tripId, linkId)
       await load()
+      return true
+    } catch (e: unknown) {
+      setError(readError(e))
+      return false
     } finally {
       setBusy(null)
     }
@@ -249,23 +291,26 @@ export function useDocSync(tripId: number | string, enabled: boolean) {
 
   const syncNow = useCallback(async (linkId: number, full = false) => {
     setBusy(`sync-${linkId}`)
+    setError(null)
     try {
       const res = await docsyncApi.syncNow(tripId, linkId, full) as {
         state: string; pulled: number; pushed: number; conflicts: number; missing: number
       }
-      setLastRun(prev => ({ ...prev, [linkId]: { pulled: res.pulled ?? 0, pushed: res.pushed ?? 0 } }))
       await load()
       return res
+    } catch (e: unknown) {
+      // A refused run is a thing the person asked for and did not get — an
+      // orphaned binding answers 409 here — so it has to be said out loud
+      // rather than escaping into the `void` at the call site. After the
+      // reload, which would otherwise clear the message.
+      await load()
+      setError(readError(e))
+      return null
     } finally {
       setBusy(null)
     }
   }, [tripId, load])
 
-  const lastRunFor = useCallback(
-    (linkId: number) => lastRun[linkId] ?? { pulled: 0, pushed: 0 },
-    [lastRun],
-  )
-
   const resolveConflict = useCallback(async (itemId: number, keep: 'trek' | 'provider' | 'both') => {
     await docsyncApi.resolve(tripId, itemId, keep)
     await load()
@@ -281,7 +326,7 @@ export function useDocSync(tripId: number | string, enabled: boolean) {
     connectionFor, load,
     saveConnection, testConnection,
     loadScopes, createScope,
-    createLink, updateLink, removeLink, syncNow, resolveConflict, lastRunFor,
+    createLink, updateLink, removeLink, syncNow, resolveConflict,
   }
 }
 

server/src/nest/doc-sync/doc-sync-webhook.controller.ts

@@ -2,7 +2,10 @@ import { Controller, HttpCode, OnModuleDestroy, Param, Post, Req } from '@nestjs
 import type { Request } from 'express';
 import crypto from 'crypto';
 import { Public } from '../auth/public.decorator';
-import { WEBHOOK_NUDGE_DEBOUNCE_SECONDS } from './doc-sync.constants';
+import { ADDON_IDS } from '../../addons';
+import { AddonsService } from '../addons/addons.service';
+import { DatabaseService } from '../database/database.service';
+import { SETTING_SYNC_ENABLED, WEBHOOK_NUDGE_DEBOUNCE_SECONDS } from './doc-sync.constants';
 import { DocSyncConfigService } from './doc-sync-config.service';
 import { DocSyncService } from './doc-sync.service';
 
@@ -39,8 +42,29 @@ export class DocSyncWebhookController implements OnModuleDestroy {
   constructor(
     private readonly config: DocSyncConfigService,
     private readonly sync: DocSyncService,
+    private readonly addons: AddonsService,
+    private readonly db: DatabaseService,
   ) {}
 
+  /**
+   * The same two switches the scheduler obeys.
+   *
+   * Without them "switch document sync off" meant "stop the poll", while every
+   * provider holding a webhook kept driving full runs — an admin turning the
+   * addon off would have watched it carry on. Checked when the timer fires as
+   * well as on arrival, so a switch thrown during the debounce window still
+   * takes effect.
+   */
+  private syncIsOn(): boolean {
+    if (!this.addons.isAddonEnabled(ADDON_IDS.DOCUMENTS)) return false;
+    const killSwitch = this.db.get<{ value: string }>(
+      'SELECT value FROM app_settings WHERE key = ?', SETTING_SYNC_ENABLED,
+    )?.value;
+    // Unrecognised values mean ON: the setting is absent by default, and only an
+    // explicit 'false' stops the sync. Same rule as the job.
+    return killSwitch !== 'false';
+  }
+
   onModuleDestroy(): void {
     for (const timer of this.pending.values()) clearTimeout(timer);
     this.pending.clear();
@@ -54,6 +78,7 @@ export class DocSyncWebhookController implements OnModuleDestroy {
     // Always 200, even for an unknown token: a 404 here would let anyone probe
     // which tokens exist, and a provider that gets an error will retry anyway.
     if (!link || link.sync_enabled !== 1) return { received: true };
+    if (!this.syncIsOn()) return { received: true };
 
     const secret = this.config.webhookSecret(link);
     if (secret && !this.secretMatches(req, secret)) return { received: true };
@@ -81,6 +106,7 @@ export class DocSyncWebhookController implements OnModuleDestroy {
       this.pending.delete(linkId);
       const fresh = reload();
       if (!fresh || fresh.sync_enabled !== 1) return;
+      if (!this.syncIsOn()) return;
       void this.sync.syncLink(fresh);
     }, WEBHOOK_NUDGE_DEBOUNCE_SECONDS * 1000);
     // A pending nudge must not hold the process open at shutdown.

server/src/nest/doc-sync/doc-sync.controller.ts

@@ -325,6 +325,13 @@ export class DocSyncController {
   ) {
     const link = this.config.getLink(Number(linkId));
     if (!link || link.trip_id !== Number(tripId)) throw new HttpException('Link not found', 404);
+    // An orphaned binding stays orphaned: its credential belongs to somebody who
+    // is no longer on this trip, and a manual run would use it anyway. The
+    // scheduler already refuses it (sync_enabled is 0), so without this the
+    // button was a way around that.
+    if (link.last_sync_state === 'orphaned') {
+      throw new HttpException({ error: 'This binding lost its owner and has to be reconnected' }, 409);
+    }
     // A person asking for a run is also asking for the rows that gave up to be
     // tried once more; the scheduler gets no such reprieve.
     this.sync.retryShelvedItems(link.id);

server/src/nest/doc-sync/doc-sync.helpers.ts

@@ -288,6 +288,27 @@ export function planReconcile(input: ReconcileInput): ReconcilePlan {
       continue;
     }
 
+    /**
+     * Paired upstream, absent in TREK: the download never landed.
+     *
+     * A failed pull stores the remote's version alongside the error, so on the
+     * next run `remoteChanged` is false, there is no local file for the other
+     * branches to compare against, and the row fell through to `touch` — which
+     * refreshes it and changes nothing. Nothing anywhere planned a second
+     * attempt: the document never arrived, no retry ever happened, and pressing
+     * "Sync now" did not help either, because the plan was the same.
+     *
+     * A rejected type and an oversized file are left out on purpose. Those are
+     * not transport failures; they are answers, and they stay true until the
+     * document upstream changes — which the `remoteChanged` branch below picks
+     * up on its own.
+     */
+    if (!l && it.fileId === null && direction !== 'push'
+        && it.state !== 'rejected_type' && it.state !== 'too_large') {
+      add({ kind: 'pull_update', remote: r, itemId: it.id });
+      continue;
+    }
+
     if (remoteChanged && !isEcho && localChanged) {
       add({ kind: 'conflict', itemId: it.id, remote: r, local: l ?? null });
       continue;

server/src/nest/doc-sync/doc-sync.service.ts

@@ -363,6 +363,14 @@ export class DocSyncService {
       return 'provider_error';
     }
 
+    // What this row pointed at before, if anything: a pull_update replaces a
+    // document TREK already holds, and `createFile` only ever inserts.
+    const supersededId = itemId === null
+      ? null
+      : (this.db.connection
+          .prepare('SELECT file_id FROM document_sync_items WHERE id = ?')
+          .get(itemId) as { file_id: number | null } | undefined)?.file_id ?? null;
+
     const created = this.files.createFile(
       ctx.link.trip_id,
       { filename: storageKey, originalname: name, size: bytes, mimetype: remote.mimeType || 'application/octet-stream' },
@@ -372,6 +380,22 @@ export class DocSyncService {
       {},
     );
 
+    /**
+     * Retire the copy this one replaces.
+     *
+     * Into the trash rather than out of existence: the bytes it holds are a
+     * version somebody may still want, and TREK's own delete works the same way.
+     * Leaving it in place was worse than untidy — the row's pairing moves to the
+     * new file, so the old one is left with no sync item at all, and the very
+     * next run reads it as a document TREK gained and pushes the superseded
+     * version back up. One upstream edit then ends as two documents on both
+     * sides, and it grows with every further edit.
+     */
+    if (supersededId !== null && Number(supersededId) !== Number(created.id)) {
+      this.files.softDeleteFile(supersededId);
+      this.realtime.broadcast(ctx.link.trip_id, 'file:deleted', { fileId: supersededId });
+    }
+
     this.upsertItem(ctx.link, {
       itemId,
       remote,
@@ -407,6 +431,11 @@ export class DocSyncService {
 
     const file = this.files.getFileById(local.fileId, ctx.link.trip_id);
     if (!file) return 'not_found';
+    // Re-read rather than trust the plan: a run walks a whole folder, and a
+    // document somebody deleted in the meantime would otherwise still be
+    // uploaded — a delete that looks ignored, and a document that reappears
+    // upstream after the person watched it go.
+    if ((file as { deleted_at?: string | null }).deleted_at) return 'not_found';
 
     let sha256 = local.sha256;
     let stream: Readable;
@@ -573,22 +602,38 @@ export class DocSyncService {
     const failed = patch.state === 'error';
 
     if (patch.itemId !== null) {
-      const attempts = failed ? 1 : 0;
+      /**
+       * The attempt counter, read before it is written.
+       *
+       * Two things needed it. The backoff was computed as
+       * `backoffSeconds(curve, 1)` — always the first step, so three of the
+       * four steps in the curve were unreachable and a provider that was down
+       * got asked again at the same short interval. And the counter only ever
+       * grew: a row that failed once a month reached the limit after six months
+       * of otherwise healthy syncing and was shelved for good. A successful
+       * pass now clears it, which is what makes the limit mean "six failures in
+       * a row" rather than "six failures ever".
+       */
+      const before = (this.db.connection
+        .prepare('SELECT attempts FROM document_sync_items WHERE id = ?')
+        .get(patch.itemId) as { attempts: number } | undefined)?.attempts ?? 0;
+      const attempts = failed ? before + 1 : 0;
+
       this.db.connection
         .prepare(
           `UPDATE document_sync_items
               SET state = ?, error_code = ?, file_id = COALESCE(?, file_id),
-                  -- see the giveUp() note below: a row that has run out of
-                  -- attempts stops retrying and waits for a person
-
                   remote_id = COALESCE(?, remote_id), remote_version = COALESCE(?, remote_version),
                   remote_name = COALESCE(?, remote_name), remote_size = COALESCE(?, remote_size),
                   remote_modified_at = COALESCE(?, remote_modified_at),
                   content_sha256 = COALESCE(?, content_sha256),
                   pushed_sha256 = COALESCE(?, pushed_sha256),
-                  attempts = attempts + ?,
+                  attempts = ?,
+                  -- A row that has used up its attempts stops retrying and waits
+                  -- for a person; planReconcile skips it and a manual run clears
+                  -- the counter (see retryShelvedItems).
                   next_attempt_at = CASE
-                    WHEN ? = 1 AND attempts + 1 < ? THEN datetime('now', '+' || ? || ' seconds')
+                    WHEN ? = 1 AND ? < ? THEN datetime('now', '+' || ? || ' seconds')
                     ELSE NULL
                   END,
                   remote_missing_at = NULL,
@@ -601,7 +646,9 @@ export class DocSyncService {
           remoteId, remoteVersion,
           patch.remote?.name ?? null, patch.remote?.size ?? null, patch.remote?.remoteModifiedAt ?? null,
           patch.contentSha256 ?? null, patch.pushedSha256 ?? null,
-          attempts, failed ? 1 : 0, ITEM_MAX_ATTEMPTS, backoffSeconds(ITEM_BACKOFF_SECONDS, 1), patch.state,
+          attempts,
+          failed ? 1 : 0, attempts, ITEM_MAX_ATTEMPTS,
+          backoffSeconds(ITEM_BACKOFF_SECONDS, attempts), patch.state,
           patch.itemId,
         );
       return;
@@ -633,7 +680,7 @@ export class DocSyncService {
         remoteId, patch.remote?.name ?? null, remoteVersion, patch.remote?.size ?? null,
         patch.remote?.remoteModifiedAt ?? null, patch.contentSha256 ?? null, patch.pushedSha256 ?? null,
         patch.state, patch.errorCode ?? null, failed ? 1 : 0,
-        failed ? 1 : 0, backoffSeconds(ITEM_BACKOFF_SECONDS, 1),
+        failed ? 1 : 0, backoffSeconds(ITEM_BACKOFF_SECONDS, 1),   // a new row is always at its first failure
         patch.state,
       );
   }
@@ -681,6 +728,28 @@ export class DocSyncService {
    * integer and nothing else tied the two together. Same rule as everywhere in
    * this codebase: every referenced id must exist AND belong to the same trip.
    */
+
+  /**
+   * Rename the provider's copy, outside a run.
+   *
+   * Only `resolveConflict` needs this: every other rename is planned and
+   * executed by a run. A failure is not fatal here — the row is left for the
+   * next run to sort out rather than the owner's choice being refused.
+   */
+  private async renameRemoteTo(link: LinkRow, remoteId: string, name: string, itemId: number): Promise<void> {
+    const conn = this.config.getConnection(link.connection_id);
+    const provider = this.registry.get(link.provider_id);
+    if (!conn || !provider) return;
+    const res = await provider.rename(this.config.toRef(conn), this.config.toScopeRef(link), remoteId, name);
+    if (docFailed(res)) {
+      this.logger.warn(`link ${link.id}: renaming ${remoteId} to "${name}" failed: ${res.error.code}`);
+      return;
+    }
+    this.db.connection
+      .prepare('UPDATE document_sync_items SET remote_name = ?, remote_version = ?, last_seen_at = CURRENT_TIMESTAMP WHERE id = ?')
+      .run(name, res.data.remoteVersion, itemId);
+  }
+
   async resolveConflict(itemId: number, keep: 'trek' | 'provider' | 'both', tripId?: number): Promise<boolean> {
     const item = this.db.connection
       .prepare('SELECT * FROM document_sync_items WHERE id = ?')
@@ -690,6 +759,27 @@ export class DocSyncService {
     const link = this.config.getLink(Number(item.link_id));
     if (!link) return false;
 
+    /**
+     * The name has to be decided too, not only the bytes.
+     *
+     * The only conflict the planner can actually raise today is a double
+     * rename, and that is arbitrated entirely by `remote_name` — the name both
+     * sides last agreed on. Clearing a version marker leaves all three names
+     * exactly as they were, so the next run compared them again, found both
+     * sides changed again, and wrote the conflict straight back. Whatever the
+     * owner picked, the issue reappeared within minutes, forever.
+     *
+     * So each choice moves the arbiter to the side that won: keeping TREK's
+     * name makes the agreed name TREK's (the provider then reads as the one
+     * that renamed, and gets renamed back), keeping the provider's takes the
+     * local rename back (TREK then reads as unchanged, and follows).
+     */
+    const localName = item.file_id === null
+      ? null
+      : (this.db.connection
+          .prepare('SELECT original_name FROM trip_files WHERE id = ?')
+          .get(item.file_id) as { original_name: string } | undefined)?.original_name ?? null;
+
     if (keep === 'trek') {
       // Forget the provider's version marker, so the next run sees no upstream
       // change and pushes TREK's copy. Clearing content_sha256 instead would do
@@ -698,10 +788,25 @@ export class DocSyncService {
       this.db.connection
         .prepare("UPDATE document_sync_items SET state = 'pending', remote_version = NULL, error_code = NULL WHERE id = ?")
         .run(itemId);
+      // Carry the name over as well, here rather than by moving the arbiter:
+      // TREK winning means the provider's copy takes TREK's name, and the only
+      // way to say that through `remote_name` would be to write the provider's
+      // CURRENT name into it — which this method does not know without asking.
+      if (localName && item.remote_id && localName !== item.remote_name) {
+        await this.renameRemoteTo(link, String(item.remote_id), localName, itemId);
+      }
     } else if (keep === 'provider') {
       this.db.connection
         .prepare("UPDATE document_sync_items SET state = 'pending', content_sha256 = NULL, error_code = NULL WHERE id = ?")
         .run(itemId);
+      // Take the local rename back to the agreed name. Only the provider's
+      // rename is then left standing, and the next run follows it the ordinary
+      // way — through the planner, with no special case in it.
+      if (item.file_id !== null && item.remote_name && localName !== item.remote_name) {
+        this.db.connection
+          .prepare('UPDATE trip_files SET original_name = ? WHERE id = ?')
+          .run(item.remote_name, item.file_id);
+      }
     } else {
       // Detach the pairing and let the next run pull the provider copy as a new
       // document. Both versions survive, under two rows.

server/src/nest/doc-sync/providers/papra.client.ts

@@ -329,12 +329,27 @@ function escapeFormValue(value: string): string {
   return value.replace(/\r/g, '%0D').replace(/\n/g, '%0A').replace(/"/g, '%22');
 }
 
+/**
+ * A media type that is safe to put in a header.
+ *
+ * The value comes from `trip_files.mime_type`, which an upload can influence,
+ * and it went into the part header untouched while the filename beside it was
+ * escaped. A CR or LF in it ends the header early and lets whatever follows be
+ * read as headers of its own. Anything that is not a plain media type is
+ * replaced rather than repaired: a mangled type is not worth guessing at, and
+ * the generic one is what an unknown file gets anyway.
+ */
+function safeMimeType(raw: string): string {
+  const TOKEN = /^[A-Za-z0-9!#$&^_.+-]+\/[A-Za-z0-9!#$&^_.+-]+$/;
+  return TOKEN.test(raw) ? raw : 'application/octet-stream';
+}
+
 /** The bytes before the file in a one-field multipart body. Exported so the tests can read them. */
 export function multipartHeader(boundary: string, fileName: string, mimeType: string): Buffer {
   return Buffer.from(
     `--${boundary}\r\n` +
       `Content-Disposition: form-data; name="file"; filename="${escapeFormValue(fileName)}"\r\n` +
-      `Content-Type: ${mimeType}\r\n\r\n`,
+      `Content-Type: ${safeMimeType(mimeType)}\r\n\r\n`,
     'utf8',
   );
 }

server/src/nest/doc-sync/providers/webdav.client.ts

@@ -126,6 +126,8 @@ export interface WebdavPutOptions {
   sha256: string;
   /** Optimistic concurrency. Both products answer a mismatch with 412. */
   ifMatch?: string;
+  /** `*` refuses to create over an existing resource (RFC 9110 §13.1.2). */
+  ifNoneMatch?: string;
 }
 
 export interface WebdavPutResult {
@@ -625,6 +627,9 @@ export class WebdavClient {
     if (creds.flavor === 'nextcloud' && /^[0-9a-f]{64}$/i.test(options.sha256)) {
       headers['OC-Checksum'] = `SHA256:${options.sha256.toLowerCase()}`;
     }
+    if (options.ifNoneMatch) {
+      headers['If-None-Match'] = options.ifNoneMatch;
+    }
     if (options.ifMatch) {
       headers['If-Match'] = options.ifMatch;
     }

server/src/nest/doc-sync/providers/webdav.provider.ts

@@ -55,8 +55,16 @@ const WEBHOOK_EVENTS = [
   'OCP\\Files\\Events\\Node\\NodeRenamedEvent',
 ] as const;
 
-/** Header the subscription carries the shared secret in; the core verifies it. */
-const WEBHOOK_SECRET_HEADER = 'X-TREK-Docsync-Signature';
+/**
+ * Header the subscription carries the shared secret in.
+ *
+ * It has to be the one the endpoint reads — `doc-sync-webhook.controller.ts`
+ * looks for `x-trek-docsync-secret`. It used to say `X-TREK-Docsync-Signature`
+ * here, so every webhook Nextcloud actually sent arrived without a secret the
+ * controller could find and was silently dropped: the subscription existed, the
+ * calls arrived, and nothing ever came of them.
+ */
+const WEBHOOK_SECRET_HEADER = 'x-trek-docsync-secret';
 
 /** Where a Nextcloud connection looks for trip folders when nobody said otherwise. */
 const DEFAULT_BASE_PATH = '/TREK';
@@ -554,6 +562,11 @@ export class WebdavDocumentProvider implements DocumentProvider {
         mtimeSeconds: req.mtimeSeconds,
         sha256: req.sha256,
         ifMatch: req.expectedRemoteVersion,
+        // Creating means creating: an unconditional PUT to a name that is
+        // already taken overwrites somebody else's document without a word.
+        // Two trips bound to one folder, or a person who put a `receipt.pdf`
+        // there by hand, are enough for that to happen.
+        ifNoneMatch: req.remoteId ? undefined : '*',
       });
 
       // Best effort, and deliberately not fatal: the bytes are already stored,
@@ -796,7 +809,12 @@ export class WebdavDocumentProvider implements DocumentProvider {
   ): Promise<string | null> {
     if (remoteId.startsWith(PATH_ID_PREFIX)) {
       const relative = remoteId.slice(PATH_ID_PREFIX.length);
-      return relative ? `${resolved.rootPath}/${encodePath(relative)}` : null;
+      // The path comes out of a listing, which is the provider's word and not
+      // TREK's. A `..` in it would address a file outside the folder the trip is
+      // bound to — every other id here is opaque, this one is a path and has to
+      // be treated like one.
+      if (!relative || relative.split('/').some(seg => seg === '..' || seg === '.')) return null;
+      return `${resolved.rootPath}/${encodePath(relative)}`;
     }
 
     const cached = this.hrefCache.get(this.cacheKey(conn, scope))?.get(remoteId);

server/tests/unit/nest/doc-sync/doc-sync-webhook.controller.test.ts

@@ -16,6 +16,8 @@ vi.mock('../../../../src/db/database', () => ({
 import { DocSyncWebhookController } from '../../../../src/nest/doc-sync/doc-sync-webhook.controller';
 import type { DocSyncConfigService, LinkRow } from '../../../../src/nest/doc-sync/doc-sync-config.service';
 import type { DocSyncService } from '../../../../src/nest/doc-sync/doc-sync.service';
+import type { AddonsService } from '../../../../src/nest/addons/addons.service';
+import type { DatabaseService } from '../../../../src/nest/database/database.service';
 
 /**
  * The webhook endpoint, with both services stubbed.
@@ -69,6 +71,15 @@ const config = {
   webhookSecret: vi.fn(() => SECRET),
 };
 
+const settings = new Map<string, string>();
+const addons = { isAddonEnabled: vi.fn(() => true) };
+const db = {
+  get: vi.fn((_sql: string, key?: unknown) => {
+    const value = settings.get(String(key));
+    return value === undefined ? undefined : { value };
+  }),
+};
+
 const sync = {
   syncLink: vi.fn(async (_link: LinkRow) => ({ state: 'ok', pulled: 0, pushed: 0, conflicts: 0, missing: 0 })),
 };
@@ -76,6 +87,8 @@ const sync = {
 const controller = new DocSyncWebhookController(
   config as unknown as DocSyncConfigService,
   sync as unknown as DocSyncService,
+  addons as unknown as AddonsService,
+  db as unknown as DatabaseService,
 );
 
 /** Only what the handler reads: headers, the parsed body, and the raw bytes. */
@@ -105,6 +118,8 @@ function papraReq(payload: string, signature: string, sent = payload): Request {
 beforeEach(() => {
   vi.clearAllMocks();
   vi.useFakeTimers();
+  settings.clear();
+  addons.isAddonEnabled.mockReturnValue(true);
   config.getLinkByToken.mockImplementation((token: string) => (token === 'tok-live' ? link() : undefined));
   config.getLink.mockImplementation((id: number) => (id === 4 ? link() : undefined));
   config.webhookSecret.mockReturnValue(SECRET);
@@ -320,3 +335,39 @@ describe('a burst of nudges', () => {
     expect(sync.syncLink).not.toHaveBeenCalled();
   });
 });
+
+/**
+ * The switches an admin expects to mean "off".
+ *
+ * The scheduler obeys both the Documents addon and the app_settings kill
+ * switch; the webhook obeyed neither, so switching document sync off stopped
+ * the poll while every provider holding a webhook carried on driving full runs.
+ */
+describe('the admin switches', () => {
+  it('does nothing while the Documents addon is off', () => {
+    addons.isAddonEnabled.mockReturnValue(false);
+    expect(controller.nudge('tok-live', makeReq({ 'x-trek-docsync-secret': SECRET }))).toEqual({ received: true });
+    settle();
+    expect(sync.syncLink).not.toHaveBeenCalled();
+  });
+
+  it('does nothing while the kill switch is set', () => {
+    settings.set('docsync_sync_enabled', 'false');
+    controller.nudge('tok-live', makeReq({ 'x-trek-docsync-secret': SECRET }));
+    settle();
+    expect(sync.syncLink).not.toHaveBeenCalled();
+  });
+
+  it('drops a scheduled run when the addon goes off inside the window', () => {
+    controller.nudge('tok-live', makeReq({ 'x-trek-docsync-secret': SECRET }));
+    addons.isAddonEnabled.mockReturnValue(false);
+    settle();
+    expect(sync.syncLink).not.toHaveBeenCalled();
+  });
+
+  it('runs normally while both are on', () => {
+    controller.nudge('tok-live', makeReq({ 'x-trek-docsync-secret': SECRET }));
+    settle();
+    expect(sync.syncLink).toHaveBeenCalledTimes(1);
+  });
+});

server/tests/unit/nest/doc-sync/doc-sync.helpers.test.ts

@@ -489,3 +489,84 @@ describe('planReconcile > upstream reported as unchanged', () => {
     expect(p.massDeleteGuardTripped).toBe(true);
   });
 });
+
+/**
+ * A download that failed once must be tried again.
+ *
+ * `pull()` records the failure together with the remote's current version, so
+ * the next run computed `remoteChanged = false`; with no local file the other
+ * branches had nothing to compare and the row fell through to `touch`. Nothing
+ * in the planner ever planned a second attempt, so the document never arrived,
+ * the attempt counter never grew past one, and "Sync now" produced the same
+ * plan. The asymmetry was visible in these tests: a failed PUSH was covered,
+ * because the TREK-only branch re-plans it; a failed PULL was not.
+ */
+describe('planReconcile > a pairing whose download never landed', () => {
+  const failedPull = (over: Partial<SyncItemState> = {}) =>
+    item({ id: 40, fileId: null, remoteId: 'r1', remoteVersion: 'v1', contentSha256: null, state: 'error', ...over });
+
+  it('plans another download', () => {
+    const p = plan({
+      items: [failedPull()],
+      remote: [remote({ remoteId: 'r1', remoteVersion: 'v1' })],
+      local: [],
+    });
+    expect(p.actions).toEqual([{ kind: 'pull_update', remote: expect.objectContaining({ remoteId: 'r1' }), itemId: 40 }]);
+  });
+
+  it('does not on a push-only binding, which has no business pulling', () => {
+    const p = plan({
+      items: [failedPull()],
+      remote: [remote({ remoteId: 'r1', remoteVersion: 'v1' })],
+      local: [],
+      direction: 'push',
+    });
+    expect(p.actions.map(a => a.kind)).not.toContain('pull_update');
+  });
+
+  it('leaves a rejected type alone: that answer does not change by trying again', () => {
+    const p = plan({
+      items: [failedPull({ state: 'rejected_type' })],
+      remote: [remote({ remoteId: 'r1', remoteVersion: 'v1' })],
+      local: [],
+    });
+    expect(p.actions.map(a => a.kind)).not.toContain('pull_update');
+  });
+
+  it('leaves an oversized document alone for the same reason', () => {
+    const p = plan({
+      items: [failedPull({ state: 'too_large' })],
+      remote: [remote({ remoteId: 'r1', remoteVersion: 'v1' })],
+      local: [],
+    });
+    expect(p.actions.map(a => a.kind)).not.toContain('pull_update');
+  });
+
+  it('does try a rejected one again once the document upstream changes', () => {
+    const p = plan({
+      items: [failedPull({ state: 'rejected_type' })],
+      remote: [remote({ remoteId: 'r1', remoteVersion: 'v2' })],
+      local: [],
+    });
+    expect(p.actions.map(a => a.kind)).toContain('pull_update');
+  });
+
+  it('gives up with the rest once the attempts are spent', () => {
+    const p = plan({
+      items: [failedPull({ attempts: 6 })],
+      remote: [remote({ remoteId: 'r1', remoteVersion: 'v1' })],
+      local: [],
+      maxAttempts: 6,
+    });
+    expect(p.actions).toEqual([]);
+  });
+
+  it('leaves a healthy pairing on touch, not on a pointless re-download', () => {
+    const p = plan({
+      items: [item({ remoteId: 'r1', remoteVersion: 'v1' })],
+      remote: [remote({ remoteId: 'r1', remoteVersion: 'v1' })],
+      local: [local({ fileId: 1 })],
+    });
+    expect(p.actions.map(a => a.kind)).toEqual(['touch']);
+  });
+});

server/tests/unit/nest/doc-sync/doc-sync.service.test.ts

@@ -165,6 +165,11 @@ const files = {
       .run(tripId, file.filename, file.originalname, file.size, file.mimetype, uploadedBy || null);
     return testDb.prepare('SELECT * FROM trip_files WHERE id = ?').get(info.lastInsertRowid);
   }),
+  // Writes for real, like the two above: a double that answers but changes
+  // nothing lets a missing call pass as a passing test.
+  softDeleteFile: vi.fn((id: string | number) => {
+    testDb.prepare('UPDATE trip_files SET deleted_at = CURRENT_TIMESTAMP WHERE id = ?').run(id);
+  }),
 } as unknown as FilesService;
 
 const realtime = { broadcast: vi.fn() } as unknown as RealtimeService;
@@ -927,6 +932,69 @@ describe('DocSyncService', () => {
       expect(itemRow(itemId).state).toBe('synced');
     });
 
+    /**
+     * The double rename is the only conflict a person can actually produce, and
+     * it is arbitrated by `remote_name` alone. Clearing a version marker left
+     * all three names untouched, so the next run raised the same conflict again
+     * — whatever the owner chose, the issue came back within minutes, forever.
+     */
+    describe('a double rename', () => {
+      function seedRenameConflict(link: LinkRow) {
+        const fileId = makeFile({ name: 'Invoice.pdf' });   // renamed in TREK
+        const itemId = seedItem(link, {
+          remoteId: 'r1',
+          remoteName: 'Bill.pdf',                            // what both agreed on
+          remoteVersion: 'v1',
+          fileId,
+          state: 'conflict',
+          contentSha256: 'agreed-hash',
+        });
+        return { itemId, fileId };
+      }
+
+      it('renames the provider copy to TREK name when TREK wins', async () => {
+        const link = makeLink();
+        const { itemId } = seedRenameConflict(link);
+        provider.list.mockResolvedValue(ok(listing([
+          remoteDoc({ remoteId: 'r1', name: 'Invoice.pdf', remoteVersion: 'v2' }),
+        ])));
+
+        await service.resolveConflict(itemId, 'trek');
+
+        expect(provider.rename).toHaveBeenCalledWith(
+          expect.anything(), expect.anything(), 'r1', 'Invoice.pdf',
+        );
+        expect(itemRow(itemId).remote_name).toBe('Invoice.pdf');
+      });
+
+      it('takes the local rename back when the provider wins, so TREK follows next run', async () => {
+        const link = makeLink();
+        const { itemId, fileId } = seedRenameConflict(link);
+        provider.list.mockResolvedValue(ok(listing([
+          remoteDoc({ remoteId: 'r1', name: 'Rechnung.pdf', remoteVersion: 'v1' }),
+        ])));
+
+        await service.resolveConflict(itemId, 'provider');
+
+        const file = testDb.prepare('SELECT original_name FROM trip_files WHERE id = ?').get(fileId) as { original_name: string };
+        expect(file.original_name).toBe('Rechnung.pdf');
+      });
+
+      it('does not leave the row in conflict either way', async () => {
+        for (const keep of ['trek', 'provider'] as const) {
+          const link = makeLink();
+          const { itemId } = seedRenameConflict(link);
+          provider.list.mockResolvedValue(ok(listing([
+            remoteDoc({ remoteId: 'r1', name: 'Rechnung.pdf', remoteVersion: 'v1' }),
+          ])));
+
+          await service.resolveConflict(itemId, keep);
+
+          expect(itemRow(itemId).state, `keep=${keep} left the row in conflict`).not.toBe('conflict');
+        }
+      });
+    });
+
     it('refuses to resolve a pairing that is not in conflict', async () => {
       const link = makeLink();
       const itemId = seedItem(link, { remoteId: 'r1', state: 'synced' });
@@ -959,6 +1027,165 @@ describe('DocSyncService', () => {
    * an audit rather than by a failing test, which is why the check lives here,
    * next to the row it is about.
    */
+  /**
+   * An upstream edit replaces TREK's copy instead of joining it.
+   *
+   * `pull_update` routes into the same `pull()` as a first download, and
+   * `FilesService.createFile` only inserts — so the edit arrived as a SECOND
+   * document while the first stayed in the file manager holding the old bytes.
+   * Worse, the pairing moves to the new row, which leaves the old one with no
+   * sync item: the next run reads it as a document TREK gained and pushes the
+   * superseded version back up. One edit, two documents on each side, growing
+   * with every further edit.
+   */
+  describe('pull_update replaces rather than accumulates', () => {
+    function seedSynced(link: LinkRow): { itemId: number; fileId: number } {
+      const fileId = makeFile({ name: 'invoice.pdf' });
+      const itemId = seedItem(link, {
+        remoteId: 'r1',
+        remoteName: 'invoice.pdf',
+        remoteVersion: 'v1',
+        fileId,
+        state: 'synced',
+        contentSha256: 'agreed-hash',
+      });
+      return { itemId, fileId };
+    }
+
+    it('puts the superseded copy in the trash instead of leaving it beside the new one', async () => {
+      const link = makeLink();
+      const { fileId } = seedSynced(link);
+      provider.list.mockResolvedValueOnce(ok(listing([
+        remoteDoc({ remoteId: 'r1', name: 'invoice.pdf', remoteVersion: 'v2' }),
+      ])));
+
+      await service.syncLink(link);
+
+      const rows = fileRows();
+      const old = rows.find(r => Number(r.id) === fileId) as Record<string, unknown>;
+      const fresh = rows.find(r => Number(r.id) !== fileId) as Record<string, unknown>;
+      expect(old.deleted_at).not.toBeNull();
+      expect(fresh).toBeDefined();
+      expect(fresh.deleted_at).toBeNull();
+    });
+
+    it('leaves exactly one live document, so the file manager does not show two', async () => {
+      const link = makeLink();
+      seedSynced(link);
+      provider.list.mockResolvedValueOnce(ok(listing([
+        remoteDoc({ remoteId: 'r1', name: 'invoice.pdf', remoteVersion: 'v2' }),
+      ])));
+
+      await service.syncLink(link);
+
+      expect(fileRows().filter(r => r.deleted_at === null)).toHaveLength(1);
+    });
+
+    it('does not push the superseded copy back on the next run', async () => {
+      const link = makeLink();
+      seedSynced(link);
+      provider.list.mockResolvedValueOnce(ok(listing([
+        remoteDoc({ remoteId: 'r1', name: 'invoice.pdf', remoteVersion: 'v2' }),
+      ])));
+      await service.syncLink(link);
+
+      provider.push.mockClear();
+      provider.list.mockResolvedValueOnce(ok(listing([
+        remoteDoc({ remoteId: 'r1', name: 'invoice.pdf', remoteVersion: 'v2' }),
+      ])));
+      await service.syncLink(link);
+
+      expect(provider.push).not.toHaveBeenCalled();
+    });
+
+    it('leaves a first download alone: there is nothing to supersede', async () => {
+      const link = makeLink();
+      provider.list.mockResolvedValueOnce(ok(listing([
+        remoteDoc({ remoteId: 'r-new', name: 'fresh.pdf', remoteVersion: 'v1' }),
+      ])));
+
+      await service.syncLink(link);
+
+      expect(fileRows().filter(r => r.deleted_at === null)).toHaveLength(1);
+    });
+  });
+
+  /**
+   * The attempt counter and the backoff curve, which were both stuck.
+   *
+   * The backoff was always computed at step 1, so three of the curve's four
+   * steps were unreachable and a provider that was down got asked again at the
+   * same short interval. And the counter only ever grew: a row that failed once
+   * a month reached the limit after six months of otherwise healthy syncing and
+   * was shelved permanently. Driven here through real runs, since the write is
+   * private and a test hatch would only prove the hatch works.
+   */
+  describe('retry bookkeeping', () => {
+    /**
+     * One run in which the download fails, with the backoff window skipped.
+     *
+     * The window is real and the planner honours it, so without clearing it the
+     * second run would correctly skip the row and this would be measuring the
+     * backoff rather than the counter.
+     */
+    async function failingRun(link: LinkRow) {
+      testDb.prepare("UPDATE document_sync_items SET next_attempt_at = NULL WHERE remote_id = 'r-flaky'").run();
+      provider.list.mockResolvedValueOnce(ok(listing([
+        remoteDoc({ remoteId: 'r-flaky', name: 'flaky.pdf', remoteVersion: 'v1' }),
+      ])));
+      provider.fetch.mockResolvedValueOnce(fail('provider_error'));
+      await service.syncLink(link);
+    }
+
+    /** One run in which it works. */
+    async function goodRun(link: LinkRow) {
+      testDb.prepare("UPDATE document_sync_items SET next_attempt_at = NULL WHERE remote_id = 'r-flaky'").run();
+      provider.list.mockResolvedValueOnce(ok(listing([
+        remoteDoc({ remoteId: 'r-flaky', name: 'flaky.pdf', remoteVersion: 'v1' }),
+      ])));
+      await service.syncLink(link);
+    }
+
+    const flakyRow = () => testDb
+      .prepare("SELECT * FROM document_sync_items WHERE remote_id = 'r-flaky'")
+      .get() as Record<string, unknown>;
+
+    it('counts consecutive failures', async () => {
+      const link = makeLink();
+      await failingRun(link);
+      expect(flakyRow().attempts).toBe(1);
+      await failingRun(link);
+      expect(flakyRow().attempts).toBe(2);
+    });
+
+    it('clears the counter once a pass succeeds, so the limit means six in a row', async () => {
+      const link = makeLink();
+      await failingRun(link);
+      await failingRun(link);
+      expect(flakyRow().attempts).toBe(2);
+
+      await goodRun(link);
+
+      expect(flakyRow().attempts).toBe(0);
+    });
+
+    it('schedules each failure further out than the last', async () => {
+      const link = makeLink();
+      const waits: number[] = [];
+      for (let i = 0; i < 3; i += 1) {
+        await failingRun(link);
+        const row = flakyRow();
+        const secs = (testDb
+          .prepare("SELECT CAST((julianday(?) - julianday('now')) * 86400 AS INTEGER) AS s")
+          .get(row.next_attempt_at) as { s: number }).s;
+        waits.push(secs);
+      }
+      // Rising rather than flat: the curve used to be indexed at 1 every time.
+      expect(waits[1]).toBeGreaterThan(waits[0]);
+      expect(waits[2]).toBeGreaterThan(waits[1]);
+    });
+  });
+
   describe('resolveConflict across trips', () => {
     function conflictedItem(linkId: number, tripOfItem: number): number {
       return Number(testDb.prepare(

server/tests/unit/nest/doc-sync/papra.provider.test.ts

@@ -28,6 +28,7 @@ import {
   tagSearchQuery,
 } from '../../../../src/nest/doc-sync/providers/papra.client';
 import { PapraProvider, buildScopeKey, parseScopeKey } from '../../../../src/nest/doc-sync/providers/papra.provider';
+import { multipartHeader } from '../../../../src/nest/doc-sync/providers/papra.client';
 
 import crypto from 'node:crypto';
 import { Readable } from 'node:stream';
@@ -1059,3 +1060,30 @@ describe('the version marker', () => {
     expect(snapshotVersion(sparse)).toMatch(/^[0-9a-f]{64}$/);
   });
 });
+
+/**
+ * The media type goes into a header, so it has to survive being one.
+ *
+ * It comes from `trip_files.mime_type` — influenced by an upload — and went in
+ * untouched while the filename beside it was escaped. A CR or LF ends the
+ * header early and lets what follows be read as headers of its own.
+ */
+describe('multipart part header', () => {
+  it('keeps an ordinary media type', () => {
+    expect(multipartHeader('B', 'a.pdf', 'application/pdf').toString())
+      .toContain('Content-Type: application/pdf');
+  });
+
+  it('refuses one carrying a newline instead of writing it into the header', () => {
+    const header = multipartHeader('B', 'a.pdf', 'application/pdf\r\nX-Evil: 1').toString();
+    expect(header).not.toContain('X-Evil');
+    expect(header).toContain('Content-Type: application/octet-stream');
+  });
+
+  it('falls back for anything that is not a media type at all', () => {
+    for (const bad of ['', 'nonsense', 'a/b/c', 'a b/c']) {
+      expect(multipartHeader('B', 'a.pdf', bad).toString())
+        .toContain('Content-Type: application/octet-stream');
+    }
+  });
+});

server/tests/unit/nest/doc-sync/webdav.provider.test.ts

@@ -786,7 +786,11 @@ describe('webhooks', () => {
     expect(bodies[0].get('httpMethod')).toBe('POST');
     expect(bodies[0].get('uri')).toBe('https://trek.example/hook');
     expect(bodies[0].get('authMethod')).toBe('header');
-    expect(bodies[0].get('authData[X-TREK-Docsync-Signature]')).toBe('s3cret');
+    // The header the endpoint actually reads. It used to be registered as
+    // `X-TREK-Docsync-Signature` while doc-sync-webhook.controller looks for
+    // `x-trek-docsync-secret`, so every webhook Nextcloud sent was dropped
+    // without a trace: the subscription existed and the calls arrived.
+    expect(bodies[0].get('authData[x-trek-docsync-secret]')).toBe('s3cret');
     // A $regex filter is not merely ignored upstream: it throws inside the
     // listener and stops delivery for every other webhook on that event.
     expect(bodies.every((body) => [...body.keys()].every((key) => !key.startsWith('eventFilter')))).toBe(true);