tessera-ctl/.planning/quick/261009-p0m-sicherheitsprotokoll-und-ci-sicherheitsp/261009-p0m-REVIEW.md

---
phase: quick-261009-p0m
reviewed: 2026-10-09T21:00:00Z
depth: standard
files_reviewed: 24
files_reviewed_list:
  - .gitea/workflows/ci.yml
  - .gitea/scripts/security-scan.sh
  - .gitea/scripts/zap-baseline.sh
  - .gitea/scripts/zap-hooks.py
  - .gitleaks.toml
  - .semgrepignore
  - .gitignore
  - .dockerignore
  - apps/api/Dockerfile
  - apps/web/Dockerfile
  - apps/web/next.config.ts
  - apps/web/src/next-config.test.ts
  - apps/api/src/crypto/crypto.service.ts
  - apps/api/src/crypto/crypto.service.spec.ts
  - apps/api/src/http-setup.ts
  - apps/api/src/http-setup.spec.ts
  - apps/api/src/main.ts
  - apps/api/src/mail/mail.module.ts
  - apps/api/src/calendar/providers/exchange.provider.ts
  - apps/api/package.json
  - apps/web/package.json
  - package.json
  - pnpm-lock.yaml
  - apps/desktop/src-tauri/Cargo.lock
findings:
  critical: 0
  warning: 5
  info: 4
  total: 9
status: issues_found
---

# Quick 261009-p0m: Code Review Report

**Reviewed:** 2026-10-09
**Depth:** standard
**Files Reviewed:** 24
**Status:** issues_found

## Summary

Reviewed `git diff d15a470..HEAD` (without `pnpm-lock.yaml` details and `.planning`). The lockfiles
were only skimmed for unexpected major bumps: there are none (Next 15.5.19 -> 15.5.27, NestJS
11.1.x -> 11.2.7, nodemailer 9.0.1 -> 9.1.1, undici 7.28.0 -> 7.30.0, vitest 4.1.9 -> 4.1.11,
rustls 0.23.41 -> .45, rustls-webpki .13 -> .15; overrides all stay inside their major).

No finding blocks a push. The checks asked for came out like this.

Verified OK:
- **ci.yml.** The diff only appends. The existing jobs are untouched. The `if:` is identical to the one on
  `desktop`. `needs: publish` is correct, and nothing depends on `security`. There is no `secrets.`
  in the job, and `timeout-minutes: 30` is set.
- **security-scan.sh.** It ends in `exit 0`, the workflow step adds `|| true`, and every download is
  SHA256-checked (cached archive included) before use. Only counts and status words are logged. The tool
  config sits in `$WORK` and is removed by the `EXIT` trap. The script's own network calls go only to
  GitHub releases, ghcr.io (Trivy DB), semgrep.dev, osv.dev, npm and PyPI. It never contacts Tessera or
  alpha. The only registry it touches is the local Docker daemon, via `docker image inspect`.
- **Crypto tag check.** Every ciphertext in the repo comes from `CryptoService.encrypt`, which has used
  Node's default 16-byte tag since the first commit (9ec6313, `createCipheriv` without options). No SQL
  migration, seed or script writes ciphertext. The LDAP backfill migration only renames a column. All
  `decrypt` callers (LDAP, calendar/Exchange, SMTP, DKV, tender mailbox, Nextcloud app password, AutoDNS,
  Proxmox) therefore read 16-byte tags, so stored values on alpha/live will not fail. There is no legacy
  short-tag format.
- **Removed packages.** `@nestjs-modules/mailer`, `ews-javascript-api` and `handlebars` appear only in
  comments. Every non-relative import in `apps/api/src` is declared in `apps/api/package.json`. Under
  pnpm's strict layout the removal cannot break a hoisted import.
- **nodemailer 9.1 / undici 7.30.** These are patch/minor bumps inside the major. Usage is limited to
  `createTransport`/`sendMail`/`Transporter` and `fetch`/`request`/`Agent`. Nothing in the diff touches
  them.
- **Runtime image.** `prisma` is in `dependencies`, so `apps/api/node_modules/.bin/prisma` exists for
  `migrate-and-start.sh`. Compose healthchecks use `wget` (busybox), not npm. No repo script runs
  `npm`/`npx`/`pnpm` inside the api or web container. `docs/anleitung-betrieb.md` states that
  `exec api pnpm`/`npm` no longer works.
- **Web headers vs. embedding.** The only iframes are the XFrame widget and custom modules (foreign
  origins, not affected), plus the welcome-mail preview (`srcDoc`, `sandbox=""`). There is no iframe of a
  Tessera page. The desktop app navigates its window to the server URL and does not frame it. Nothing
  uses `getUserMedia`, geolocation, `navigator.usb` or `PaymentRequest`. The only `window.open` call
  already passes `noopener,noreferrer`, so COOP `same-origin-allow-popups` is safe.
- **ZAP.** The hook turns off form processing and form POST, and there is no AJAX spider and no active
  scan. The `ghcr.io` image is pinned by digest.

## Warnings

### WR-01: Prisma client for the runtime image is generated only through a silently-failing postinstall, and nothing in CI starts the image

**File:** `apps/api/Dockerfile:33-39`, `apps/api/package.json:13`
**Issue:** The old Dockerfile ran an explicit `RUN prisma generate`, which fails the build on error. Now
the runtime client only comes from the `postinstall` of `apps/api` during `pnpm install --prod`
(`prisma generate || true`). If the engine download or the generate step fails, the error is swallowed.
The image builds, `publish` pushes it, and it dies at start-up (`@prisma/client did not initialize yet`).
It also breaks if a pnpm update stops running workspace-project lifecycle scripts. The proof that it works
(`checks/fresh-db-start.sh`, `task5-image.txt`) lives in untracked `.planning/`, so nothing repeats it
after this commit. The CI `publish` job builds the image but never starts it.
**Fix:**
```dockerfile
RUN pnpm install --frozen-lockfile --prod --filter=@tessera/api...
# Hard-fail if the client was not generated (postinstall swallows errors with `|| true`)
RUN apps/api/node_modules/.bin/prisma generate --schema apps/api/prisma/schema.prisma \
 && find node_modules/.pnpm -path '*/.prisma/client/index.js' | grep -q .
```
Also commit `fresh-db-start.sh` (e.g. under `.gitea/scripts/`) and call it after the image build, or at
least keep it reachable from the release checklist in the Betriebsanleitung.

### WR-02: Semgrep is installed from PyPI without hash pinning, in a job that has the host Docker socket

**File:** `.gitea/scripts/security-scan.sh:254-270`
**Issue:** gitleaks, Trivy and osv-scanner are SHA256-pinned. Semgrep is only version-pinned:
`pipx install semgrep==1.180.0` resolves its transitive dependencies freely. A compromised PyPI
dependency would execute in a job container that mounts `/var/run/docker.sock` (see
`docker-compose.ci.yml`), which is root on the host. The install lands in the persistent
`/opt/hostedtoolcache/tessera-security`. On later runs only `--version | grep -q "^1.180.0"` is checked,
and that executes the cached binary. The plan and docs claim "pinned and checksum-verified scanners";
this one is not. "No secret in the job" limits what leaks, but not the host exposure.
**Fix:** Install from a hash-locked requirements file (`pip install --require-hashes -r semgrep-1.180.0.txt`
into a venv). Alternatively run Semgrep as a container pinned by digest. Do not trust the cached
`semgrep` beyond a version string.

### WR-03: "Never fails and never delays the pipeline" is not guaranteed, and was not exercised on the real Gitea

**File:** `.gitea/workflows/ci.yml:266-295`
**Issue:**
1. Job-level `continue-on-error` and `timeout-minutes` are honored by GitHub, but Gitea Actions support is
   version-dependent. The SUMMARY says the first real CI run is still pending. The script itself cannot
   fail, so the remaining ways to turn the run red are `actions/checkout`, the runner, and the 30-minute
   timeout. `docs/ci-cd-setup.md` already concedes "Job ist rot oder gelb".
2. The runner is documented as the single runner. Every push to `main` therefore holds it for up to 30
   minutes after `publish`. The next push's `quality`/`test` waits in the queue, which contradicts the
   plan truth "never ... delays quality, test, desktop or publish".

**Fix:** After the first push, check that the run shows green/neutral and not red (`security` red =
unsupported). If the single runner is the bottleneck, move the scan to a scheduled workflow
(`on: schedule`, nightly) plus tags `v*`, or cut the job budget (see WR-04). Keep the existing
`if: gitea.ref == ...` line.

### WR-04: Per-tool timeouts add up to far more than the job timeout, and the summary is only printed at the very end

**File:** `.gitea/scripts/security-scan.sh:264,286,298-300,333,359,375,403,424,583-597`
**Issue:** The sum of the limits is about 600 (pipx) + 120 (corepack) + 3 x 600 (curl) + 900 (gitleaks) +
300 (audit) + 600 (osv) + 1500 (semgrep) + 900 (trivy fs) + 2 x 900 (images), roughly 8,500 s, against
`timeout-minutes: 30` (1,800 s). A slow Semgrep or Trivy run makes the runner kill the whole job before
`summarize` runs. The log then holds no `SECURITY-SUMMARY` lines and the report directory may not be
uploaded. The documented guarantee "the SECURITY-SUMMARY lines in the log are always enough" fails in
exactly the slow case.
**Fix:** Either print a one-line count right after each tool, or keep one global deadline. For example,
compute `DEADLINE=$(( $(date +%s) + 1500 ))` and let `tmo` use `min(limit, DEADLINE-now)`. Also lower
Semgrep to about 600 s.

### WR-05: ZAP basic-auth replacer rule is not scoped to the target and can send alpha credentials to other hosts

**File:** `.gitea/scripts/zap-hooks.py:23-32`
**Issue:** With `ZAP_BASIC_AUTH_FILE` set, a global replacer rule (`initiators=""`, no URL restriction)
overwrites `Authorization` on every request that ZAP sends. That includes any redirect target, and ZAP's
own add-on/update traffic if it is enabled, not only requests to alpha. The credentials are the Basic
Auth of the test server. The same file sits in the report (documented), so the report must not be passed
on. The rule itself adds a second exposure. It only matters when the manual run needs the creds, but it
is avoidable.
**Fix:** Scope the rule to the target, e.g. pass `url=re.escape(target) + '.*'` to `replacer.add_rule`
(the `url` parameter exists in current Replacer API versions), and use `ZAP_TARGET` from `zap_started`'s
`target` argument.

## Info

### IN-01: Hard-coded Yarn path makes the hardening silently disappear on a base-image bump

**File:** `apps/api/Dockerfile:52-53`, `apps/web/Dockerfile:47-48`
**Issue:** `rm -rf ... /opt/yarn-v1.22.22 ...` names one exact version. `node:24-alpine` floats; when the
image ships a different Yarn, `rm -rf` of a missing path succeeds and Yarn (and its transitive
advisories) remain with no build failure. The same line is copied into two Dockerfiles.
**Fix:** `rm -rf /opt/yarn-v* ...` and add `RUN ! command -v npm && ! command -v yarn && ! command -v corepack && node --version`
so the build fails if the removal stops working.

### IN-02: Headers duplicated by the proxy may conflict, contrary to the comment in `next.config.ts`

**File:** `apps/web/next.config.ts:31-40`
**Issue:** The comment says that if the reverse proxy sets the same headers "nothing is gained or lost".
That is true only for identical values. If NPM later adds `X-Frame-Options: DENY` or its own CSP,
browsers treat two conflicting values as the stricter one (or ignore XFO), which could block Tessera
being framed by itself or produce two CSPs. The first public ZAP run (still outstanding per the
protocol) is the point to check it.
**Fix:** After the public-URL ZAP run, record that the proxy sends none of these headers. Leave the
comment otherwise as is, but drop the "nothing lost" claim.

### IN-03: `zap-baseline.sh` leaves the report folder world-writable and breaks on credentials with quotes

**File:** `.gitea/scripts/zap-baseline.sh:92,111`
**Issue:** `chmod 777 "$REPORT_DIR"` is done so the container user (uid 1000) can write, but the report
can contain Basic Auth credentials (documented). Any local user can read it. Separately,
`printf 'user = "%s"\n'` makes curl's config parser stop on a `"` or `\` in the password, which then
aborts (exit 4, fail-closed, but confusing).
**Fix:** Use `chmod 770` and `chown`/`--user "$(id -u):$(id -g)"` for the container instead of 777, and
escape `\` and `"` in `CRED` before writing the curl config.

### IN-04: Shallow clone would silently shorten the gitleaks history scan

**File:** `.gitea/scripts/security-scan.sh:325-337`
**Issue:** `ci.yml` uses `fetch-depth: 0`, but the script has no check. Running it from a shallow local clone
still reports `gitleaks findings=0` for the visible commits only.
**Fix:** `git -C "$ROOT" rev-parse --is-shallow-repository` and emit
`SECURITY-SUMMARY gitleaks ... note=flaches-repository` when true.

---

_Reviewed: 2026-10-09_
_Reviewer: Claude (gsd-code-reviewer)_
_Depth: standard_

## Fix status

Fixed 2026-10-09 in commits `bf632d9` (code) and `8a1218a` (docs). Not pushed.

| Finding | Status | What was done / proof |
|---------|--------|-----------------------|
| WR-01 | fixed | `apps/api/Dockerfile` (stage `prod-deps`): explicit `prisma generate` plus a check that `.prisma/client/index.js` exists, no `|| true`. Proof: a throwaway Dockerfile with a corrupted `schema.prisma` built the install step (postinstall swallowed the error) and then FAILED at the new step (rc 1). Fresh-DB proof script moved to `.gitea/scripts/image-start-check.sh` (versioned, documented in Entwicklungsanleitung and Betriebsanleitung Kap. 9); on the rebuilt image: `migrationen=63`, `frische-datenbank ok`. CI jobs untouched. |
| WR-02 | fixed | Semgrep runs from `semgrep/semgrep@sha256:529ee8a2...` (digest pinned, `docker create` + `docker cp` in/out, because the job path is not a host path). pipx/PyPI path removed. Exit-0 and SHA256 checks for the other tools unchanged. |
| WR-03 | open, by design | Verify after the push: job `security` shows green/neutral (not red) on the real Gitea, `continue-on-error` and `timeout-minutes` honoured, and the single runner is not blocked (see SUMMARY checklist). Recorded as open row in the protocol. |
| WR-04 | fixed | Global budget `BUDGET_SECONDS=1500`; per-tool limits sum to 1485 s (install 375, checks 1110); `tmo` clamps to remaining time; each tool prints its `SECURITY-SUMMARY` line immediately and appends to `summary.txt`; reasons `zeitgrenze` / `gesamtbudget`. Proven with forced limits (semgrep 2 s, trivy 1 s, budget 6 s). Full run in the runner image: 63 s, rc 0. |
| WR-05 | fixed | Replacer rule gets `url=<escaped target>(?:[/?#].*)?`. Proof through a ZAP daemon proxy: target `127.0.0.1:2000` got the header; `127.0.0.1:20001` (same host, prefix port) and `127.0.0.2:2002` (second host) did not. |
| IN-01 | fixed | `rm -rf /opt/yarn* /usr/local/bin/yarn* ...` plus a loop `command -v` over npm/npx/corepack/yarn/yarnpkg and an `ls` absence check, in both Dockerfiles. Proof: a Dockerfile that leaves npm in place fails the build. |
| IN-02 | fixed | Comment in `apps/web/next.config.ts` now says differing proxy values apply side by side (stricter wins) and points to the public ZAP run. `next-config.test.ts` 4/4, biome clean. |
| IN-03 | fixed | Report dir `chmod 700` (uid 1000 case); other uids hand the dir over to 1000 and back via a short container run (path exercised with a faked uid). Credentials only as base64 in a 0600 header file (`curl -H @file`) and env file; no curl config string. Proof: password with `"` and `\` passes the pre-check, ZAP run 0 POST, 8 of 9 requests with header (the 9th is the pre-check). |
| IN-04 | fixed | `git rev-parse --is-shallow-repository`; gitleaks line reads `findings=0 unvollstaendig (flacher Klon: ...)`. Proven with a `--depth 1` clone. |