docs(18): Protokoll der Review-Korrekturen

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

.planning/phases/18-desktop-client-fertigstellen/18-REVIEW-FIX.md

@@ -0,0 +1,85 @@
+---
+phase: 18-desktop-client-fertigstellen
+fixed_at: 2026-09-16T17:40:00Z
+review_path: .planning/phases/18-desktop-client-fertigstellen/18-REVIEW.md
+iteration: 1
+findings_in_scope: 4
+fixed: 4
+skipped: 2
+status: all_fixed
+verification_env: main checkout (workflow.use_worktrees=false, no isolated worktree used)
+---
+
+# Phase 18: Code Review Fix Report
+
+**Fixed at:** 2026-09-16T17:40:00Z
+**Source review:** `.planning/phases/18-desktop-client-fertigstellen/18-REVIEW.md`
+**Iteration:** 1
+
+**Summary:**
+- Findings in scope (Critical + Warning): 4
+- Fixed: 4
+- Skipped (Info, out of scope by instruction): 2
+
+Verification ran directly in the main checkout at `/home/vicolab/projects/tessera-ctl` — `.planning/config.json` has `workflow.use_worktrees: false`, so no isolated git worktree was created for this run; per the fixer's setup rules this is the documented, safe opt-out path.
+
+## Fixed Issues
+
+### CR-01: Path-traversal defense-in-depth regex accepts dot-only filenames
+
+**Files modified:** `apps/api/src/desktop/desktop.service.ts`, `apps/api/src/desktop/desktop.service.spec.ts`
+**Commit:** `0d5c80f`
+**Applied fix:** In `getPackage()` step (4), `entry.name === '.'` and `entry.name === '..'` are now rejected explicitly (the character-class regex alone accepted them since `.` and `-` are both allowed characters). Additionally, the resolved absolute path is now checked to still start with the resolved `desktopDistDir` before any filesystem access, as a second, independent layer of defense against future variants of this pattern if the character whitelist is ever reused elsewhere. Added `desktop.service.spec.ts` Test 7a (`entry.name: '..'` → 404), Test 7b (`entry.name: '.'` → 404), and Test 7c (`entry.name: '../manifest.json'` → 404, matching the exact case named in the fix task).
+**Verification:** Tier 1 (re-read, clean) + Tier 2 (`vitest run src/desktop`: 13/13 passing; `tsc --noEmit`: clean).
+
+### WR-01: Tauri CSP grants `'unsafe-eval'` and wildcard sources that are never needed
+
+**File modified:** `apps/desktop/src-tauri/tauri.conf.json`
+**Commit:** `1b2f803`
+**Applied fix:** Tightened `app.security.csp` from `default-src 'self' 'unsafe-inline' 'unsafe-eval'; connect-src *; img-src * data:; font-src * data:; style-src 'self' 'unsafe-inline' *; script-src 'self' 'unsafe-inline' 'unsafe-eval'` to `default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self'` — exactly the value REVIEW.md suggested. Confirmed by reading `setup.html`: it uses no `eval()`, no remote fonts/images/styles, and talks to the Rust side only via `window.__TAURI__.core.invoke` (IPC bridge, not `fetch()`). Confirmed via Tauri knowledge that `app.security.csp` is injected only into responses served by the app's own asset protocol (the bundled frontend, i.e. `setup.html`) — the `window.navigate()` call in `save_server_url()` that follows loads the user's configured server fresh, governed by that server's own response headers, not by this config, so tightening `connect-src` here does not affect the subsequently loaded remote page.
+**Verification:** Tier 1 (re-read, clean) + Tier 2 (`node -e "JSON.parse(...)"`: valid JSON).
+
+### WR-02: Desktop update check ignores commit/channel — beta users between tags never see "update available"
+
+**Files modified:** `apps/desktop/src-tauri/build.rs`, `apps/desktop/src-tauri/src/lib.rs`
+**Commit:** `579e24b`
+**Applied fix:** `build.rs` now runs `git rev-parse --short=7 HEAD` at compile time and embeds the result as `APP_COMMIT` via `cargo:rustc-env` (falls back to an empty string if `git` is unavailable, e.g. a source tarball without `.git`). This mirrors exactly the format `desktop-collect.sh` already writes into `manifest.json`'s `commit` field. `lib.rs`'s `DesktopLatest` struct now also deserializes `channel` and `commit` (both already present in every `/desktop/latest` response per `DesktopLatestResponse`); the update-available check is now `info.version != app_version || (info.channel == "beta" && info.commit != app_commit)`, so beta clients see the notice for a newer commit on the same tag-derived version, while the live channel keeps the plain version comparison. D-07 (plain `X.Y.Z` in `tauri.conf.json`/`Cargo.toml`, unaffected by this change) is untouched — `desktop-version.sh` was not modified.
+**Verification:** Tier 1 (re-read, clean) + Tier 2 (`cargo check`: clean; `cargo clippy -- -D warnings`: clean, forced re-run via `touch src/lib.rs`).
+**Note:** This is a logic-level fix to an async comparison with no existing Rust unit tests in this crate to exercise it automatically (only `cargo check`/`clippy`, which verify syntax/lints, not runtime behavior). Per the fixer's verification policy for logic findings, **this one requires human/manual verification** before relying on it — e.g. building a beta package, bumping only the commit (not the tag-derived version), and confirming the tray notice now appears. Flagged in `18-REVIEW.md`.
+
+### WR-03: `getManifest()` shallow-validates `files`; malformed entries fall through to string-coerced lookups
+
+**Files modified:** `apps/api/src/desktop/desktop.service.ts`, `apps/api/src/desktop/desktop.service.spec.ts`
+**Commit:** `a8964f1`
+**Applied fix:** `getManifest()`'s top-level shape check now also rejects `files` being an array (`Array.isArray(parsed.files)`, since `typeof [] === 'object'` previously slipped through). A new `isValidManifestFileEntry()` helper validates each present platform entry has `name: string`, `size: number`, and `sha256: string` matching a 64-character hex pattern (`/^[a-f0-9]{64}$/i`) — stricter than REVIEW.md's minimum suggestion (which only asked for the three `typeof` checks), matching the fix-task's explicit scope instruction to also validate the sha256 hex format. A malformed entry makes the whole manifest treated as missing (`null` return, same 404 path, plus a `logger.warn`), matching the docstring's stated guarantee that a bad manifest shape means 404. Added Test 9 (manifest with `name` missing on the `linux` entry → `getLatest()` throws `NotFoundException` instead of proceeding to a stringified-`undefined` lookup) and Test 10 (`sha256: 'not-a-hash'` → download 404).
+**Verification:** Tier 1 (re-read, clean) + Tier 2 (`vitest run src/desktop`: 13/13 passing; `tsc --noEmit`: clean).
+
+## Skipped Issues
+
+### IN-01: Redundant/duplicated version-format validation in `desktop-collect.sh`
+
+**File:** `.gitea/scripts/desktop-collect.sh:66-77`
+**Reason:** Info-severity finding, explicitly out of scope for this fix run per the fix task's scope instruction ("Skip the two Info findings (document as skipped)"). No code change made.
+**Original issue:** The `case` glob pattern is immediately followed by a strict `grep -qE` doing the actual validation; the glob branch adds no protection the grep doesn't already provide.
+
+### IN-02: `apps/desktop/src-tauri/capabilities/default.json` grants `opener:allow-open-url` for any http/https URL
+
+**File:** `apps/desktop/src-tauri/capabilities/default.json:17`
+**Reason:** Info-severity finding, explicitly out of scope for this fix run. The finding itself also states "no action required unless the opener use expands to cover more than the update link" — not a code change candidate even under broader scope.
+**Original issue:** Capability allows opening any http/https URL in the system browser; currently only used for the tray "Update herunterladen" link to the user-configured server, consistent with D-02 (no fixed built-in server) and not expressible more tightly in the static capability file.
+
+## Gate Results
+
+| Gate | Result |
+|------|--------|
+| `pnpm --filter @tessera/api exec vitest run src/desktop` | 13/13 passed |
+| `pnpm --filter @tessera/api type-check` | clean (`tsc --noEmit`, no errors) |
+| `cargo check` (`apps/desktop/src-tauri`) | clean |
+| `cargo clippy -- -D warnings` (`apps/desktop/src-tauri`) | clean, no warnings |
+| `pnpm --filter @tessera/web exec vitest run` | not run — no web files touched by any of the four fixes |
+
+---
+
+_Fixed: 2026-09-16T17:40:00Z_
+_Fixer: Claude (gsd-code-fixer)_
+_Iteration: 1_