From 4fa06c34834d337b1dbd86644ed4ac1497dbedad Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 19 Sep 2026 00:07:33 +0200 Subject: [PATCH] fix(docsync): an idle WebDAV binding read as an emptied folder, and two ids nobody checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit's worst find, reproduced against a live Nextcloud before and after the fix: the WebDAV adapters answer "nothing changed here" by returning an EMPTY listing plus `cursorUnchanged`, and nothing read that flag — `grep -rn cursorUnchanged src/` found it set by four adapters and consulted by none. The core therefore saw a folder that had lost everything. Run 1 and 2 look fine because the run's own uploads move the ETag; from run 3 on, every run of an idle binding ends `partial` with `mass_delete_guard` and six documents reported missing, forever, until failure_count opens the circuit and the scheduler drops the binding entirely. That is the ordinary case — a binding nobody is touching. planReconcile now takes `remoteUnchanged` and plans only what happened in TREK. Two ids were authorised by trip and never checked against it: - resolveConflict took an item id on trust, so the owner of any trip could decide a conflict belonging to somebody else's. Both are plain integers and nothing compared them. - The connection test merged the STORED credential into a probe of whatever baseUrl the caller named — a way to have TREK post another account's API token at a server of the caller's choosing. Stored secrets are now reused only for the address they were stored against. The phone's connect form was wrong in three ways at once, each of which the desktop had right: it read the address as `baseUrl` while the field is `base_url` (so every connection it saved carried an empty address), it drew `allow_insecure_tls` as a text box and always sent false, and it built label keys as `docsync.provider${cap(label)}` when the label column already holds the full suffix — `docsync.providerProviderApiKey`, rendered raw on screen. Those rules are not markup, so they now live in `useConnectForm`, which both shells use. Its test fixture invented the field shape and so agreed with the broken sheet; it now mirrors document_provider_fields. Also: the scheduler read its interval once at bootstrap while this file's own comment promised a per-tick read, so changing it did nothing until a restart; and dueLinks ordered by a column that a successful run clears, which left every binding tied and SQLite answering in insertion order — on a database with 170 bindings the same twenty ran on every tick and nothing created after them was ever synced automatically. --- .../Files/docsync/DocSyncConnectModal.tsx | 49 ++--- .../Files/docsync/useConnectForm.ts | 109 +++++++++++ .../screens/trip/tabs/MDocSyncSheet.test.tsx | 21 +- .../screens/trip/tabs/MDocSyncSheet.tsx | 73 ++++--- .../src/nest/doc-sync/doc-sync.controller.ts | 23 ++- server/src/nest/doc-sync/doc-sync.helpers.ts | 31 +++ server/src/nest/doc-sync/doc-sync.job.ts | 36 +++- server/src/nest/doc-sync/doc-sync.service.ts | 29 ++- .../nest/doc-sync/doc-sync.controller.test.ts | 17 ++ .../nest/doc-sync/doc-sync.helpers.test.ts | 90 +++++++++ .../unit/nest/doc-sync/doc-sync.job.test.ts | 180 ++++++++++++++---- .../nest/doc-sync/doc-sync.service.test.ts | 72 +++++++ 12 files changed, 608 insertions(+), 122 deletions(-) create mode 100644 client/src/components/Files/docsync/useConnectForm.ts diff --git a/client/src/components/Files/docsync/DocSyncConnectModal.tsx b/client/src/components/Files/docsync/DocSyncConnectModal.tsx index e5cd07836..e2a8f8c96 100644 --- a/client/src/components/Files/docsync/DocSyncConnectModal.tsx +++ b/client/src/components/Files/docsync/DocSyncConnectModal.tsx @@ -3,6 +3,7 @@ import { AlertTriangle, Check, Loader2, ShieldAlert } from 'lucide-react' import Modal from '../../shared/Modal' import ToggleSwitch from '../../Settings/ToggleSwitch' import { useTranslation } from '../../../i18n/TranslationContext' +import { useConnectForm } from './useConnectForm' import { Badge } from './DocSyncBits' import { DOCUMENT_PROVIDER_ICONS } from '../../shared/DocumentProviderIcons' import type { DocSyncProvider, useDocSync } from './useDocSync' @@ -30,35 +31,13 @@ export default function DocSyncConnectModal({ const existing = sync.connectionFor(provider.id) const Icon = DOCUMENT_PROVIDER_ICONS[provider.id] - const [values, setValues] = useState>({}) - const [insecure, setInsecure] = useState(null) + const form = useConnectForm(provider, existing, sync) const [verdict, setVerdict] = useState<{ connected: boolean; account?: string; error?: string } | null>(null) - // Null until touched, so a connection that loads after the dialog opened is - // still reflected. Seeding the state at mount showed a stored "allow - // self-signed" as off and wrote that back on the next save — a security - // switch silently turning itself off is the worst version of this bug. - const insecureOn = insecure ?? existing?.allowInsecureTls ?? false - - const baseUrl = values.base_url ?? existing?.baseUrl ?? '' - // The address is a column of its own, not a settings entry, so it has to be - // read from there — otherwise the field sits empty while the form submits the - // stored value behind it. - const shown = (key: string) => - values[key] ?? (key === 'base_url' ? existing?.baseUrl : existing?.settings?.[key]) ?? '' - const required = provider.fields.filter(f => f.required) - const canSubmit = required.every(f => - f.field_key === 'base_url' - ? baseUrl.trim().length > 0 - : f.secret - ? (values[f.field_key] ?? '').length > 0 || !!existing?.secrets?.[f.field_key] - : shown(f.field_key).trim().length > 0, - ) - - const test = async () => setVerdict(await sync.testConnection(provider.id, baseUrl, values, insecureOn)) + const test = async () => setVerdict(await form.probe()) const save = async () => { - if (await sync.saveConnection(provider.id, baseUrl, values, insecureOn)) onConnected(provider.id) + if (await form.save()) onConnected(provider.id) } return ( @@ -79,7 +58,7 @@ export default function DocSyncConnectModal({