--- phase: 261008-mzu-modul-nextcloud-dateien-eigenstaendiger- reviewed: 2026-10-08T20:18:02Z depth: standard files_reviewed: 54 files_reviewed_list: - apps/api/prisma/migrations/20261008180000_nextcloud_files/migration.sql - apps/api/prisma/migrations/20261008183000_nextcloud_files_login_name/migration.sql - apps/api/src/nextcloud-files/dto/nextcloud-files-connect.dto.ts - apps/api/src/nextcloud-files/dto/nextcloud-files-ops.dto.ts - apps/api/src/nextcloud-files/dto/nextcloud-files-settings.dto.ts - apps/api/src/nextcloud-files/dto/nextcloud-files-transfer.dto.ts - apps/api/src/nextcloud-files/nextcloud-auth-client.ts - apps/api/src/nextcloud-files/nextcloud-call-gate.ts - apps/api/src/nextcloud-files/nextcloud-dav-transfer.ts - apps/api/src/nextcloud-files/nextcloud-dav.ts - apps/api/src/nextcloud-files/nextcloud-files-account.service.ts - apps/api/src/nextcloud-files/nextcloud-files.controller.ts - apps/api/src/nextcloud-files/nextcloud-files.module.ts - apps/api/src/nextcloud-files/nextcloud-files.seed.ts - apps/api/src/nextcloud-files/nextcloud-files.service.ts - apps/api/src/nextcloud-files/nextcloud-files-settings.service.ts - apps/api/src/nextcloud-files/nextcloud-files-transfer.service.ts - apps/api/src/nextcloud-files/nextcloud-files.types.ts - apps/api/src/nextcloud-files/nextcloud-http.ts - apps/api/src/nextcloud-files/nextcloud-login-guard.ts - apps/api/src/nextcloud-files/nextcloud-propfind.ts - apps/api/src/nextcloud-files/nextcloud-server-info.ts - apps/api/src/nextcloud-files/nextcloud-upstream.ts - apps/web/src/app/(portal)/modules/nextcloud-files/layout.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/page.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/AccountBar.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/Breadcrumb.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/ConnectPanel.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/DeleteDialog.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/Dialog.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/DropOverlay.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/EntryMenu.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/FileBrowser.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/FileGrid.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/FileList.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/icons.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/MoveDialog.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/NameDialog.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/QuotaMeter.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/SelectionBar.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/ServerIdentity.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/SettingsTab.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/Toolbar.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/TransferBar.tsx - apps/web/src/app/(portal)/modules/nextcloud-files/components/TypeTile.tsx - apps/web/src/components/nextcloud-files/drop-entries.ts - apps/web/src/components/nextcloud-files/error-text.ts - apps/web/src/components/nextcloud-files/file-format.ts - apps/web/src/components/nextcloud-files/file-types.ts - apps/web/src/components/nextcloud-files/paths.ts - apps/web/src/components/nextcloud-files/selection.ts - apps/web/src/components/nextcloud-files/use-transfers.ts - apps/web/src/lib/nextcloud-files-api.ts - apps/web/src/lib/nextcloud-files-upload.ts findings: critical: 2 warning: 9 info: 7 total: 18 status: issues_found fix_status: all_fixed fixed_at: 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 `NextcloudFilesAccount` includes 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`, `nosniff` and a `sandbox` CSP. - 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: 1. **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. 2. **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. 3. **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 test` passed 2961/2961; `pnpm --filter @tessera/web test` passed 1648/1648. - Type checks: `tsc --noEmit` is clean in api and in web. - Lint: biome on all touched files shows only 4 pre-existing `noNonNullAssertion` warnings in old spec lines. - Stack: `docker compose up -d --build api web` succeeded; 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. ```ts // 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`), 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`: 1. Accepts any http or https host (internal addresses are allowed by design). 2. Marks every account EXPIRED, so all users are pushed back to the connect screen with the "connection expired" notice. 3. 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: ```ts @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 `` 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: ` 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: ```ts 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: 1. The user drops folder `A`. 2. The user deletes or renames `A` in the browser. 3. The user drops a folder named `A` again. `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 `` (`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=`. 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`: ```ts 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_