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

phase, fixed_at, review_path, iteration, findings_in_scope, fixed, skipped, status, verification_env

phase	fixed_at	review_path	iteration	findings_in_scope	fixed	skipped	status	verification_env
18-desktop-client-fertigstellen	2026-09-16T17:40:00Z	.planning/phases/18-desktop-client-fertigstellen/18-REVIEW.md	1	4	4	2	all_fixed	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