32 KiB
phase, reviewed, depth, files_reviewed, files_reviewed_list, findings, status, fix_status, fixed_at
| phase | reviewed | depth | files_reviewed | files_reviewed_list | findings | status | fix_status | fixed_at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 261008-mzu-modul-nextcloud-dateien-eigenstaendiger- | 2026-10-08T20:18:02Z | standard | 54 |
|
|
issues_found | all_fixed | 2026-10-08 |
Quick 261008-mzu: Code Review Report
Reviewed: 2026-10-08T20:18:02Z Depth: standard (focus areas 1 to 3 traced in full; presentational web components only skimmed) Files Reviewed: 54 Status: issues_found
Summary
The transport containment is solid. Every Nextcloud call goes through ncRequest, which builds the URL from the stored base, a fixed list of path prefixes and segments that are validated and encoded one by one. It never follows a redirect and never calls poll.endpoint or login from a Login Flow answer. Cookies are never sent. Destination is always built from the session base. .., /, \, NUL and control characters are rejected per segment, and an encoded slash cannot get through because each segment goes through encodeURIComponent.
Secrets are handled correctly:
- The real password lives in exactly one call.
- The app password is stored with AES-GCM encryption and is decrypted only in
getSession. - No view, log line or error body carries a secret.
- RLS on
NextcloudFilesAccountincludes the user dimension, and every account query is bound to the tenant and user from the token. - 401 and 403 from Nextcloud are always mapped to 409 or 422.
- Streamed downloads use a header allowlist and add their own
Content-Disposition: attachment,nosniffand asandboxCSP. - The logo is identified from its first bytes and served with a sandbox CSP.
- File names and theming values are rendered as React text, and the theming color is limited to
#rrggbb. - Routes without a parameter are declared before the routes that take one.
The problems are in concurrency and authorization scope:
- Brute-force limits can be bypassed with parallel requests (CR-01). The password-attempt limits are checked before the Nextcloud call and recorded after it. Parallel requests therefore get past both the per-user and the server-wide limit, which defeats the control that is supposed to keep Nextcloud from locking out the shared Tessera IP.
- Non-admin managers control where passwords are sent (CR-02). Anyone with the module grant "Verwalten" can point the module at any host. Every user must then reconnect, and they type their real password into a screen showing a name and logo that the attacker controls.
- Smaller gaps: Several issues weaken app-password hygiene (no revoke during a pause), conditional-replace semantics (TOCTOU on chunked replace, wrong 412 mapping) and the upload and ZIP paths.
Fix Summary (2026-10-08, gsd-code-fixer)
All 18 findings are fixed; IN-02 was handled as the trivial variant (no SVG previews). Twelve commits on main, not pushed:
| Commit | Findings |
|---|---|
5c2d327 |
CR-01 |
e78e059 |
CR-02 |
74e086f |
WR-01 |
c0b8283 |
IN-03 |
ddae940 |
WR-02, IN-04 |
07474bd |
WR-03 |
ee05131 |
WR-04, WR-05, WR-06, IN-01 |
08abfef |
WR-09, IN-02 |
486819f |
WR-07 |
8679668 |
WR-08, IN-06 |
f0f3718 |
IN-05 |
49eebde |
IN-07 |
Verification ran in the main checkout (workflow.use_worktrees=false):
- Tests:
pnpm --filter @tessera/api testpassed 2961/2961;pnpm --filter @tessera/web testpassed 1648/1648. - Type checks:
tsc --noEmitis clean in api and in web. - Lint: biome on all touched files shows only 4 pre-existing
noNonNullAssertionwarnings in old spec lines. - Stack:
docker compose up -d --build api websucceeded; the api had freshly restarted before the e2e runs. - End-to-end: all four scripts pass against tessera-nc-test (after
nc-test-setup.sh):e2e-settings.sh,e2e-connect.sh,e2e-files.sh,e2e-transfer.sh. - Browser: a Chromium (Playwright) check of the real UI confirmed the selection ZIP via form POST into a hidden iframe through
/api-proxy, and the download precheck message.
Critical Issues
CR-01: Password-attempt limits are check-then-act across an await and can be bypassed with parallel requests
Fix status: fixed in 5c2d327. beginPasswordAttempt reserves the attempt synchronously in both the per-user and the server-wide window before the Nextcloud call. release frees the slot on success or on a non-credential failure. fail keeps it for a 401 or a timeout. Connect and store run serialized per tenant and user (withUserLock, also around the Login Flow store), so the second of two parallel successful connects revokes the first app password as the "previous" one. Specs use concurrent Promise.all: 5 parallel wrong passwords → 3 reach Nextcloud and 2 get 429; 10 users → 8 reach Nextcloud; two successful connects → one row and exactly the unstored password revoked.
File: apps/api/src/nextcloud-files/nextcloud-files-account.service.ts:160-169, apps/api/src/nextcloud-files/nextcloud-login-guard.ts:179-204
Issue: connectWithPassword calls guard.checkPasswordAttempt(userId, scope) synchronously, then await getAppPassword(...), and only after the Nextcloud answer calls guard.recordFailure. While a request is still waiting for Nextcloud, nothing counts it. Any authenticated module user can send N parallel POST connect/password with a wrong password, and all N pass the check:
- The per-user limit (3 in 15 minutes) does not hold.
- The server-wide limit (8 in 30 minutes) does not hold.
- All N attempts reach Nextcloud as failed logins from the single Tessera IP.
That is exactly the lockout for all users that the guard exists to prevent (L-04). The same race lets two concurrent successful connects both run revokePrevious on the same old row. Each then writes its own row, so one freshly issued app password is never stored and never revoked.
Fix: Reserve the slot before the await and release it on success. In-flight attempts then count against both limits.
// login-guard
beginPasswordAttempt(userId: string, scope = ''): () => void {
this.checkPasswordAttempt(userId, scope); // throws 429 if full
const now = this.now();
const mine = this.userFailures.get(userId) ?? []; mine.push(now); this.userFailures.set(userId, mine);
const server = this.serverFailures.get(scope) ?? []; server.push(now); this.serverFailures.set(scope, server);
return () => { // call on success or on a non-credential failure
remove(mine, now); remove(server, now);
};
}
// account service
const release = this.guard.beginPasswordAttempt(userId, scope);
const issued = await getAppPassword(...);
if (!issued.ok && issued.kind === 'credentials') { /* keep the reservation */ throw ... }
release();
Additionally, serialize connectWithPassword and the store part of pollFlow per user (an in-process Map<userId, Promise>), so that revokePrevious and upsert cannot interleave.
CR-02: A non-admin with the "Verwalten" grant can redirect every user's Nextcloud password to a host they control
Fix status: fixed in e78e059. PUT settings now carries @Roles(ADMIN, SUPER_ADMIN) without @ModuleManage, the same pattern as TendersController.getSourceConfig. POST settings/test stays at Verwalten, but non-admins may only test the saved address; any other address returns 403. The web SettingsTab is read-only for non-admins: no Save button and a short note. The "not configured" hint points non-admins to an administrator. Updated: metadata spec (module-manage-handlers), controller spec, page tests, e2e-settings.sh (MANAGE user gets 403 on PUT and on testing a different address, 200 on testing the saved one), Anleitung Administration/Anwender and CHANGELOG. No audit event was added; that part was optional.
File: apps/api/src/nextcloud-files/nextcloud-files.controller.ts:132-144, apps/api/src/nextcloud-files/nextcloud-files-settings.service.ts:115-147
Issue: PUT settings is protected only by @ModuleManage('nextcloud-files'). The guard grants that to administrators and to any user with the module grant level MANAGE (module.guard.ts:111). Saving a new baseUrl:
- Accepts any http or https host (internal addresses are allowed by design).
- Marks every account EXPIRED, so all users are pushed back to the connect screen with the "connection expired" notice.
- Shows the new server's
productname, theming color and logo there (nextcloud-server-info.ts), which the attacker controls.
The next POST connect/password sends Basic base64(loginName:realPassword) to that host (nextcloud-auth-client.ts:210-214). In this deployment, Nextcloud passwords are typically the AD/LDAP passwords. So a delegated module manager (not a Tessera admin) can collect the directory credentials of every colleague who reconnects. The review brief assumes the base URL is "admin-set", but the code does not enforce that.
Fix: Restrict changing the address to Tessera administrators. Keep GET settings and POST settings/test at MANAGE if wanted, but put a role check on PUT settings. The class comment explains why @Roles must not be added (the global RolesGuard would lock out managers), so do the check inside the handler:
@Put('settings')
@ModuleManage('nextcloud-files')
async saveSettings(@Req() req: AuthenticatedRequest, @Body() dto: SaveNextcloudFilesSettingsDto) {
if (req.user?.role !== 'ADMIN') throw ncErrorDefault('notAllowed'); // 422, not 403
return this.settings.saveSettings(this.requireTenantId(req), dto);
}
Also consider recording an audit event with the old and new host when the address changes.
Warnings
WR-01: The address test follows up to 3 redirects to arbitrary hosts and returns HTTP status details, which works as an internal SSRF probe
Fix status: fixed in 74e086f. testAddress now calls status.php through ncRequest: no redirects, call gate applies, no credentials. Results are coarse only: ok, maintenance, paused, locked, redirect, not-nextcloud, unreachable. They carry no HTTP status, error code or redirect target. NEXTCLOUD_STATUS_FETCHER was removed. Together with CR-02, only admins can probe new addresses.
File: apps/api/src/nextcloud-files/nextcloud-files-settings.service.ts:157-187, 245-248; apps/api/src/nextcloud-status/nextcloud-status-fetch.ts:221-247
Issue: testAddress (called by POST settings/test and after every saveSettings) reuses fetchNextcloudStatus. That function follows up to MAX_REDIRECTS = 3 Location headers to any http or https host. It does not go through ncRequest or the call gate. This contradicts the module rule "never follow redirects, never call URLs from a Nextcloud response". The response also separates timeout, network, tls, not-nextcloud and http-status (HTTP xxx), so a MANAGE user (see CR-02) can scan internal hosts and ports, including through redirects from an external host they control.
Fix: For the module's own test, call /status.php through ncRequest (prefix: '/status.php'; any 3xx gives redirect). If the "address redirects" hint is wanted, report kind: 'redirect' together with the Location origin as text, without fetching it. Return a coarse result (ok, notNextcloud, unreachable) instead of the raw HTTP status.
WR-02: A freshly issued or old app password is not revoked when the origin is paused or the network fails
Fix status: fixed in ddae940. Revokes that fail temporarily (paused/429, network, timeout, maintenance, 5xx) go into an in-memory queue capped at 200 entries and 6 attempts. They are retried 1 s after the pause ends, or after 60 s. Secrets are never logged and are only sent to their own base URL. Fresh, previous and disconnect revokes all go through this queue. Specs cover a 429 on cloud/user, a network error on the old password, giving up after 6 attempts, and no retry on 401.
File: apps/api/src/nextcloud-files/nextcloud-files-account.service.ts:178-181, 254-256, 386-388, 393-414, 416-445; apps/api/src/nextcloud-files/nextcloud-http.ts:317-320
Issue: revokeFresh and revokeBestEffort call ncRequest, and ncRequest returns paused before the transport whenever the origin is on the call gate. The most likely reason for getCurrentUser to fail right after getAppPassword succeeded is a 429, which pauses the origin. The revoke is then never sent. The result is an app password that is valid in Nextcloud and stored nowhere, which contradicts D-P ("wird sofort widerrufen"). The same applies to revokePrevious during a pause or a network error: the old password is overwritten in the database but stays valid in Nextcloud. Failures are only logged as a warning.
Fix: Let revoke calls bypass the origin pause (a single DELETE with a known-good credential does not feed brute-force detection). Alternatively, put failed revokes into a small in-memory retry queue that is drained after the pause ends. At minimum, do not overwrite the old row while its revoke failed; keep the encrypted old value in a pendingRevoke column.
WR-03: The dead-credential short-circuit does not stop the first wave of parallel 401s
Fix status: fixed in 07474bd. The gate holds at most 4 concurrent calls per credential key until the response headers arrive (acquireSlot). Waiting calls re-check dead/paused before sending, and an abort while waiting returns aborted. Chunk PUT, single PUT and assembly are unthrottled, because their headers only arrive after the full body or after minutes. Spec: 20 parallel previews with a revoked key → at most 4 transport calls. Web: previews were already loading="lazy". A failed preview now triggers a quiet re-list at most every 10 s, and on connectionExpired the page returns to the connect screen.
File: apps/api/src/nextcloud-files/nextcloud-http.ts:314-316, 394-398; apps/web/src/app/(portal)/modules/nextcloud-files/components/TypeTile.tsx:66-70; apps/web/src/app/(portal)/modules/nextcloud-files/components/FileGrid.tsx:24-27
Issue: markDead runs only after the first 401 has come back. The list and grid views render one <img src=preview> per image row, and many of them load in parallel when the user scrolls or opens a folder (HTTP/2 through the proxy imposes no per-host limit of 6). If the app password is revoked mid-session (for example, deleted in the Nextcloud device list), every preview request in flight reaches Nextcloud before the first 401 returns. Each one counts as a failed login for the shared Tessera IP; the code comment itself says Nextcloud counts every 401 this way. One user scrolling a photo folder can therefore push the server towards the lockout of 10 failures in 30 minutes.
Fix: Allow only one outstanding Nextcloud request per credential key until a key has been proven alive once in the process (record the first 2xx per key). Alternatively, use a per-key semaphore (for example 4) for preview only, or let preview wait for a single in-flight "probe" per key.
WR-04: The chunked "replace" is check-then-act: the ETag probe and the MOVE with Overwrite: T are not atomic
Fix status: fixed in ee05131. Measured against Nextcloud 34 (tessera-nc-test): If-Match on the chunked-v2 assembly MOVE is evaluated against the source .file, so every If-Match, including the correct ETag, returns 412. It cannot be used. Fallback: the ETag re-check runs inside the assembly job immediately before the MOVE and fails with changedMeanwhile. A residual window of milliseconds between the PROPFIND and the MOVE remains; this is documented in code.
File: apps/api/src/nextcloud-files/nextcloud-files-transfer.service.ts:393-406, 413-417; apps/api/src/nextcloud-files/nextcloud-dav-transfer.ts:411-416
Issue: For a replace, completeUpload checks the ETag with PROPFIND and then sends MOVE .file with Overwrite: T and without If-Match. If another client changes the target between the probe and the move (and assembly of large files can take minutes), that version is silently overwritten. This is exactly what "replace only the version the user saw" is supposed to prevent. The single-PUT path does it correctly with If-Match.
Fix: Send If-Match: <replaceEtag> on the assembly MOVE as well (Nextcloud evaluates the conditional headers against the destination on chunked-v2 assembly) and map a 412 to changedMeanwhile. Keep the probe only as an early check.
WR-05: A concurrent or retried complete with replaceEtag runs the assembly twice and can report a successful upload as failed
Fix status: fixed in ee05131. The assembly record is set synchronously before the first await after the dedup check. A second or concurrent complete replays the stored result: assembling, done, or the same error with the same status. It never re-assembles. Only failures before the MOVE was sent (precheck error, paused) are retryable. Specs cover parallel completes, replay after a 507, and a retry after a precheck network error.
File: apps/api/src/nextcloud-files/nextcloud-files-transfer.service.ts:385-409
Issue: The deduplication check (known?.state === 'assembling') runs before await dav.stat(...), but the assembling record is created only after it. Two complete calls for the same uploadId with replaceEtag (a client retry after a network error, or a second tab) both get past the check and both send the MOVE. The second MOVE fails (404, the upload folder has already been consumed). Its fresh record replaces the first one in the map. uploadState then reports failed although the file was written, and the web shows an error.
Fix: Write the assembling record before the first await that follows the dedup check, and remove it if the replace probe rejects:
const record: AssemblyRecord = { state: 'assembling' };
this.assemblies.set(key, record);
try { if (dto.replaceEtag) { /* probe */ } } catch (e) { this.assemblies.delete(key); throw e; }
WR-06: A 412 on a single PUT with If-Match is reported as nameTaken, so "Replace" can loop
Fix status: fixed in ee05131. A 412 with replaceEtag maps to changedMeanwhile (with existing) on the single PUT and on the assembly MOVE.
File: apps/api/src/nextcloud-files/nextcloud-files-transfer.service.ts:304-307; apps/api/src/nextcloud-files/nextcloud-upstream.ts:494-495
Issue: codeForStatus(412) always returns nameTaken. With replaceEtag set, a 412 means "the version changed meanwhile", not "the name is taken". The web shows the conflict prompt again. If the user chooses "Replace" again, the process repeats with the newer ETag. That ETag may differ from what the user meant to replace, which silently weakens the safeguard.
Fix: In putSingle, if query.replaceEtag is set and the status is 412, throw changedMeanwhile with existing. The web already clears replaceEtag on changedMeanwhile retry (use-transfers.ts:271).
WR-07: The folder-creation cache in the transfer queue never expires and skips MKCOL for folders that were deleted later
Fix status: fixed in 486819f. The created-folders cache is cleared on a new drop while nothing is active, by forgetFolders() after delete/move/rename in FileBrowser, and when an upload fails with pathConflict/notFound ("Erneut versuchen" then re-creates the folder). Within one batch each folder is still created only once. A new hook test covers this (use-transfers.test.ts).
File: apps/web/src/components/nextcloud-files/use-transfers.ts:105, 116-143
Issue: folders.current keeps the resolved promise of every created path for the whole lifetime of the component. Consider this sequence:
- The user drops folder
A. - The user deletes or renames
Ain the browser. - The user drops a folder named
Aagain.
ensureFolders then finds the cached promise and does not send MKCOL. Every file upload into /A/... fails with pathConflict (409).
Fix: Cache only in-flight promises: delete the entry in a finally once it settles. Alternatively, clear folders.current in enqueue whenever no transfer is active, and after any delete or move action.
WR-08: Multi-selection ZIP puts every name into the query string and breaks on realistic selections
Fix status: fixed in 8679668. POST download/zip takes the names in the body: JSON, or the web's hidden form with names as JSON text. At most 1000 names. The web submits a form into a hidden iframe, so the stream goes straight to disk and cookie auth and Content-Disposition are kept. Verified in Chromium through /api-proxy: the ZIP is downloaded and the page stays. If everything in the folder is selected, the web downloads the whole-folder ZIP. Additional finding: Nextcloud accepts the selection only in its own URL (files=), and Apache in front of it rejects request lines over 8190 chars with 414 (measured: about 6150 chars pass, 8200 fail). The API therefore returns 413 selectionTooLarge when the upstream URL would exceed 8000 chars, also on the precheck. The web warns before submitting (encoded budget 7000), with a clear message. In practice this caps a selection at roughly 100–200 names, not 1000. e2e-transfer.sh was moved to POST (JSON, form via /api-proxy, 300 long names → 413, old GET → 404).
File: apps/web/src/lib/nextcloud-files-api.ts:276-279; apps/web/src/app/(portal)/modules/nextcloud-files/components/FileBrowser.tsx:438-451; apps/api/src/nextcloud-files/dto/nextcloud-files-transfer.dto.ts:203-209
Issue: zipUrl encodes each selected name as name=.... The API allows up to 500 names of 255 characters each, and the UI allows selecting up to 5000 entries. Typical selections fail in the following ways:
- A few hundred files with ordinary names already exceed Node's 16 KiB request-header limit (431) or the URI buffer of Nginx Proxy Manager (414).
- More than 500 names produce a 400 from the ValidationPipe.
Because the download is started from a hidden <a> (triggerDownload), the user gets a failed download or a JSON error file and no message.
Fix: Cap the selection client-side (for example 200 names or 8 KiB of query string) with a clear message. Better, add a POST download/zip-token that stores the selection server-side for 60 s and returns a short id for GET download/zip?t=<id>. If everything in a folder is selected, fall back to the whole-folder ZIP.
WR-09: A lone UTF-16 surrogate in a path makes encodeURIComponent throw and returns an unmapped 500
Fix status: fixed in 08abfef. validateSegment rejects lone UTF-16 surrogates, so the result is 400 invalidPath/invalidName instead of a URIError 500. Valid surrogate pairs (emoji) still work. The web never retried 4xx; a test now pins isRetryable(400) to false.
File: apps/api/src/nextcloud-files/nextcloud-http.ts:111-129, 167-182
Issue: JSON bodies (POST folders, POST move, POST uploads, POST uploads/:id/complete) can carry "\uD800". validateSegment accepts it, and encodeSegments then throws URIError: URI malformed inside buildNcUrl. Nothing catches this, so Nest answers with a generic 500 instead of the { code: 'invalidPath' } contract. A 500 without code also counts as retryable in nextcloud-files-upload.ts:90-93, so the client retries three times for nothing.
Fix: Reject lone surrogates in validateSegment:
if (!segment.isWellFormed()) throw ncErrorDefault('invalidPath'); // in validateSegment
(Node 24 has String.prototype.isWellFormed.)
Info
IN-01: replaceEtag is not validated before it is used as an If-Match header
Fix status: fixed in ee05131. @Matches(ETAG_PATTERN) was added on replaceEtag in StartUploadDto, CompleteUploadDto and UploadQueryDto. A value with CR/LF, spaces, non-ASCII or over 128 chars → 400.
File: apps/api/src/nextcloud-files/dto/nextcloud-files-transfer.dto.ts:128-132, 177-180; apps/api/src/nextcloud-files/nextcloud-dav-transfer.ts:324
Issue: A value containing CR, LF or other invalid header characters makes undici throw. classifyTransportError then reports it as network, which surfaces as 504 nextcloudUnavailable (misleading).
Fix: Add @Matches(/^(W\/)?"?[\x21\x23-\x7e]{1,128}"?$/) to the DTO.
IN-02: Previews are served inline and accept image/svg+xml
Fix status: fixed (trivial) in 08abfef. Previews accept raster image/* only; image/svg+xml → notFound, so the web falls back to the type tile. CSP sandbox and nosniff stay. Cross-Origin-Resource-Policy: same-origin was deliberately not added: in the dev setup the web (:3000) loads previews from the API (:3001), which is cross-origin, and that header would break previews there.
File: apps/api/src/nextcloud-files/nextcloud-files.service.ts:393-402
Issue: The default-src 'none'; sandbox CSP and nosniff neutralize scripts, so this is defense-in-depth only.
Fix: Reject image/svg+xml in accept, or add Content-Disposition: inline; filename="preview" together with Cross-Origin-Resource-Policy: same-origin.
IN-03: http base URLs are accepted silently
Fix status: fixed in c0b8283. The settings show a visible warning box whenever the address starts with http://: passwords and app passwords travel unencrypted. Anleitung Administration mentions it. The address is still accepted, by design.
File: apps/api/src/nextcloud-files/nextcloud-files-settings.service.ts:119
Issue: With an http:// address, both the real password (getapppassword) and every app password travel as cleartext Basic auth.
Fix: Show a warning in the settings check when the scheme is http: and the host is not loopback.
IN-04: Cancelling a browser login does not stop Nextcloud from issuing the token
Fix status: fixed in ddae940. Cancelled, replaced and address-change flows stay as cancelled tombstones until they expire. They do not count against the 200 open flows and are capped at 500. The server polls them every 10 s (sweepCancelledFlows, unref'd timer). If the login is granted after all, the issued app password is revoked immediately and never stored. A poll that sees an address change now cancels instead of removing.
File: apps/api/src/nextcloud-files/nextcloud-files-account.service.ts:274-279
Issue: If the user finishes the browser login after pressing "Cancel", Nextcloud still creates an app password, which is never polled and never revoked.
Fix: Keep a cancelled entry as a tombstone and keep polling it until it expires. If it is granted, revoke the password.
IN-05: Browser login can report "expired" although the account was connected
Fix status: fixed in f0f3718. On a 404/410 from the poll, ConnectPanel first reloads the status through onConnected and only then shows flowExpired. If the account is in fact connected, the files view appears instead.
File: apps/api/src/nextcloud-files/nextcloud-files-account.service.ts:237-239; apps/web/src/app/(portal)/modules/nextcloud-files/components/ConnectPanel.tsx:183-186
Issue: The flow entry is removed before getCurrentUser and the store step. If the response that carries connected is lost (proxy timeout during slow Nextcloud calls), the next poll gets 404. The UI then shows flowExpired while the account is in fact connected.
Fix: On 404 or 410, call onConnected() (which reloads the status) before showing the error.
IN-06: Download errors are invisible
Fix status: fixed in 8679668. Every download (file, folder ZIP, selection ZIP, and clicking a file name) first runs a cheap precheck: check=1 / check: true, one PROPFIND Depth 0. Errors appear in the status line ("Herunterladen nicht möglich: …"); connectionExpired returns to the connect screen. Verified in Chromium against the stack: a deleted file shows "Herunterladen nicht möglich: Der Eintrag existiert nicht mehr."
File: apps/web/src/app/(portal)/modules/nextcloud-files/components/FileBrowser.tsx:74-83
Issue: Downloads use a hidden anchor. connectionExpired, nextcloudLocked and notFound end in a failed browser download with no message, and an expired connection does not bring the connect screen back.
Fix: Optionally run a cheap HEAD or stat before downloading, or accept this as a known limitation.
IN-07: The window-level drop handler ignores open dialogs, menus and defaultPrevented
Fix status: fixed in 49eebde. While a dialog or menu is open, or the target is inside [role=dialog|alertdialog|menu], the window-level handler shows no overlay and uploads nothing. It still calls preventDefault, so the browser does not open the file itself. Drops that were already defaultPrevented are ignored.
File: apps/web/src/app/(portal)/modules/nextcloud-files/components/FileBrowser.tsx:401-414
Issue: Dropping a file onto the rename input or onto an open dialog backdrop starts an upload into the current folder.
Fix: Return early when dialog !== null, when e.defaultPrevented is set, or when the target is inside [role="dialog"].
Reviewed: 2026-10-08T20:18:02Z Reviewer: Claude (gsd-code-reviewer) Depth: standard