Files
Aether/BEFUNDE.md
T
marcandClaude Opus 5 2cc02941b9 fix(files): beschaedigte Downloads werden gemeldet und abgebrochen
Die Pruefsumme wurde beim Ausliefern mitgerechnet und das Ergebnis
weggeworfen -- ein gekipptes Bit wurde erkannt und verschwiegen.

Jetzt: Logeintrag mit Datei-ID, Speicherschluessel, erwarteter und
gelesener Summe, dazu panic(http.ErrAbortHandler), den chis Recoverer
ausdruecklich durchreicht.

Damit das wirkt, werden die letzten 32 KiB zurueckgehalten. Sonst waere die
Datei beim Erkennen laengst vollstaendig beim Empfaenger und der Abbruch
kaeme zu spaet. Erst wenn die Summe stimmt, darf der Schluss hinterher;
sonst bekommt die Gegenstelle weniger Bytes als angekuendigt und wertet das
als Fehler statt als Datei.

scripts/pruefsumme-selbsttest.py, 14 Pruefungen: kippt ein Bit auf der
Platte (nicht im Programm), in der Mitte der Datei, damit es nicht schon an
der Laenge auffaellt; zwei Groessen ueber und unter den 32 KiB; und zuerst
die Gegenprobe, dass eine heile Datei durchkommt.

**Dabei gefunden:** Der neue Logeintrag blieb aus, obwohl der Abbruch
funktionierte. zerolog.Ctx liefert einen *abgeschalteten* Logger, wenn im
Kontext keiner steckt -- und Aether legte nie einen hinein. Alle 34
log.Ctx-Aufrufe im Baum schrieben ins Nichts, darunter die Warnung ueber
verwaiste Blobs beim Zurueckrollen einer Fassung.

Behoben zweifach: zerolog.DefaultContextLogger beim Start (deckt auch
Hintergrundarbeit ab) und ein Logger mit request_id im Anfragekontext. Zwei
Tests halten es fest.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-12 22:15:22 +02:00

229 lines
10 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Bestandsaufnahme, 12.09.2026
Einmal durch die ganze Cloud: Code, Sicherheit, offene Enden. Geordnet nach
dem, was passieren kann — nicht nach Aufwand.
Was hier steht, ist **gemessen**, nicht geschätzt: Routen aus `main.go`
extrahiert und gegen die Rechteprüfungen in den Handlern abgeglichen,
`govulncheck` über den ganzen Baum, Zeilen und Testumfang je Paket gezählt.
---
## 1 · Ein Bild-Upload kann den Prozess töten
**Schwere: hoch. Jeder Kontoinhaber, normale Bedienung.**
`cmd/aether/main.go:702` erzeugt das Vorschaubild in einer **nackten
Goroutine**:
```go
go func() {
…
thumbnails.Generate(ctx, st, …) // dekodiert eine hochgeladene Datei
}()
```
Chis `Recoverer` hängt am Anfragebehandler. Eine Panik in einer *eigenen*
Goroutine fängt er nicht — sie nimmt den **ganzen Go-Prozess** mit. Docker
startet ihn neu (`restart: unless-stopped`), aber jeder gerade laufende Upload
stirbt mit, und wer die Datei noch einmal hochlädt, wiederholt das Spiel.
Die passende Lücke war offen: **GO-2026-5061**, Panik im WebP-Dekodierer bei
unstimmiger Alphakanal-Größe. Behoben am 12.09.2026 durch `x/image` 0.42 → 0.45
(`ef60497`).
**Damit ist die Hälfte erledigt, nicht das Problem.** Die Bauweise bleibt: Der
nächste Dekodierer-Fehler trifft genauso, und Bilddekodierer sind eine Gattung,
in der regelmäßig etwas gefunden wird. Vier weitere Goroutinen dieser Art gibt
es: `internal/sharing/handler.go:232` und `main.go:185` (beide nur Datenbank,
harmlos), `internal/gc/gc.go:257`, und der OCR-Arbeiter (`go worker.Run(ctx)`),
der Fremdprogramme aufruft und deren Ausgabe liest.
**✅ Erledigt am 12.09.2026.** `pkg/bg` mit zwei Funktionen:
* `bg.Go(name, fn)` – startet eine Goroutine, die eine Panik überlebt.
* `bg.Run(name, fn)` – dasselbe ohne eigene Goroutine, für Schleifen.
Der Unterschied ist keine Feinheit: Fängt man eine Panik um den **ganzen**
Arbeiter herum ab, überlebt der Prozess – aber der Arbeiter ist tot, und von da
an wird still keine Datei mehr erkannt. Der OCR-Arbeiter sichert deshalb jeden
**Auftrag** einzeln ab; ein unverdauliches Dokument kostet dann genau sich
selbst.
Umgestellt: Vorschaubild, Bestand nachtragen, Freigabe-Zähler,
Speicherbereinigung, OCR-Arbeiter. Eine nackte Goroutine bleibt mit Begründung
im Code – die Signalbehandlung beim Herunterfahren; ein recover brächte dort
nichts, weil `main` ohnehin wartete, und sie fasst keine fremden Daten an.
Eine aufgefangene Panik wird mit Stapel und Herkunftsname protokolliert. Sie
ist ein Fehler und bleibt einer – sie reißt nur nichts mehr mit. Vier Tests in
`pkg/bg/bg_test.go`, darunter der, auf den es ankommt: Eine Schleife läuft nach
einer Panik im dritten Durchgang weiter.
> Der Unterschied ist wichtig: Die Version zu heben schloss **diese** Lücke.
> Der `recover` schließt die **Gattung**.
---
## 2 · Beschädigte Downloads werden erkannt und verschwiegen
**Schwere: mittel. Datenintegrität, still.**
`internal/files/handler.go:496` rechnet beim Ausliefern die Prüfsumme mit —
genau so, wie es die Projektregel verlangt („SHA-256: beim Upload berechnen,
beim Download verifizieren"). Nur passiert bei einer Abweichung nichts:
```go
actual := hex.EncodeToString(h256.Sum(nil))
if actual != f.Checksum {
// Log corruption; headers already sent, can't change status.
// TODO: alert / observability hook
_ = actual
}
```
Kein Logeintrag, kein Zähler, keine Meldung. Der Kommentar sagt, was fehlt.
Dass der Statuscode nicht mehr zu ändern ist, stimmt — die Kopfzeilen sind
längst draußen. Aber **still** muss es deshalb nicht sein. Ein `log.Error()`
mit Datei-ID, erwarteter und tatsächlicher Summe kostet nichts und ist der
Unterschied zwischen „ich habe eine kaputte Datei" und „ich habe eine kaputte
Datei und weiß es seit drei Monaten nicht".
Zweitens ließe sich die Verbindung abbrechen, statt sie sauber zu beenden: Der
Browser zeigt dann einen abgebrochenen Download statt einer heilen Datei mit
falschem Inhalt. Das ist die ehrlichere Ausgabe.
**✅ Erledigt am 12.09.2026.** Beides – und ein Drittes, das dabei herauskam.
Der Logeintrag nennt Datei-ID, Speicherschlüssel, erwartete und gelesene Summe.
Dazu wird die Verbindung fallen gelassen (`panic(http.ErrAbortHandler)`, den
chis Recoverer ausdrücklich durchreicht).
**Damit das überhaupt wirkt, mussten die letzten 32 KiB zurückgehalten
werden.** Sonst wäre die Datei beim Erkennen des Fehlers längst vollständig
beim Empfänger, und der Abbruch käme zu spät: Die Kopfzeilen sind draußen, die
Bytes auch. Ein `tailWriter` reicht deshalb alles durch außer dem Schluss; erst
wenn die Summe stimmt, darf der hinterher. Stimmt sie nicht, bekommt die
Gegenstelle weniger Bytes als im `Content-Length` angekündigt – und wertet das
als Fehler statt als Datei.
Geprüft in `scripts/pruefsumme-selbsttest.py` (14 Prüfungen): Der Test kippt
ein Bit **auf der Platte**, nicht im Programm, und zwar in der Mitte der Datei,
damit der Fehler nicht schon an der Länge auffällt. Zwei Größen – über und
unter den 32 KiB, denn bei einer kleinen Datei wird gar nichts herausgegeben.
Und er prüft zuerst, dass eine **heile** Datei vollständig durchkommt; ohne das
bewiese der Rest nichts.
---
## 2b · Und der Fund dahinter: 34 Logaufrufe schrieben ins Nichts
**Schwere: hoch, indirekt. Ein Fehler, der andere Fehler versteckt.**
Der neue Logeintrag blieb beim ersten Testlauf aus, obwohl der Abbruch
funktionierte. Ursache:
```go
func Ctx(ctx context.Context) *Logger {
if l, ok := ctx.Value(ctxKey{}).(*Logger); ok { return l }
else if l = DefaultContextLogger; l != nil { return l }
return disabledLogger // ← hier landete alles
}
```
`zerolog.Ctx` liefert einen **abgeschalteten** Logger, wenn im Kontext keiner
steckt und kein Standard gesetzt ist. Aethers `middleware.Logger` hängte nie
einen hinein, und `DefaultContextLogger` war nie gesetzt.
**Alle 34 `log.Ctx(...)`-Aufrufe im Baum schrieben damit ins Nichts.** Darunter:
* die Warnung, dass beim Zurückrollen einer Fassung ein Blob verwaist ist
(`files/handler.go`, `files/replace.go`, `handler_versions.go`)
* gescheiterte Suchen
* Fehler der Speicherbereinigung
* die neue Meldung über beschädigte Dateien
Das ist die unangenehmste Art Fehler: Der Aufruf sieht richtig aus, `go vet`
schweigt, und auffallen kann es nur, wenn man den Fehler herbeiführt, den die
Zeile melden soll.
**Behoben zweifach:** `zerolog.DefaultContextLogger` beim Start gesetzt (deckt
auch Hintergrundarbeit mit eigenem Kontext ab), und `middleware.Logger` hängt
zusätzlich einen Logger mit `request_id` in den Anfragekontext – damit bei zwei
gleichzeitigen Downloads erkennbar bleibt, welche Meldung zu welchem gehört.
Zwei Tests in `pkg/middleware/logger_test.go` halten es fest.
---
## 3 · Vier Pakete ohne einen einzigen Go-Test
**Schwere: mittel. Nicht was heute kaputt ist, sondern was morgen still kaputtgeht.**
| Paket | Zeilen | Test | Aussenpruefung |
|---|---:|---:|---|
| `internal/tags` | 1132 | **0** | keine |
| `internal/documents` (OCR, Volltext) | 1008 | **0** | keine |
| `internal/sharing` | 382 | **0** | Python-Selbsttest, 24 Prüfungen |
| `internal/rescan` | 479 | **0** | Python-Selbsttest, 20 Prüfungen |
| `internal/audit` | 207 | **0** | keine |
| `internal/thumbnails` | 113 | **0** | keine |
Bei `sharing` und `rescan` deckt wenigstens ein Selbsttest die Außenseite ab.
Bei **`documents` und `tags` gar nichts** — und `documents` ist das Stück, das
aus `scan_0042.pdf` ein auffindbares Dokument macht. Genau der Grund, aus dem
Aether das Rechnungsarchiv ist.
**Vorschlag:** Nicht auf Abdeckung zielen, sondern auf die Stellen, an denen
ein Fehler still bleibt: Beim OCR die Warteschlange (Versuche, Aufgeben,
Wiederaufnahme nach hartem Abbruch), bei den Etiketten die Rechteprüfung —
Etiketten sind je Konto, und das prüft heute nichts nach.
---
## 4 · Was in Ordnung ist
Der Vollständigkeit halber, weil eine Liste nur aus Mängeln ein falsches Bild
gibt:
* **Alle 27 Admin-Routen prüfen serverseitig auf Adminrecht.** Zwei
Ausnahmen sind Absicht und dokumentiert: `GET /admin/settings` liest jeder
Angemeldete (Theming — und es liegen dort nur sechs Branding-Schlüssel,
nachgesehen), `GET /ocr` ist der eigene Erkennungsstand ohne Adminrecht.
* **Kein einziges zusammengesetztes SQL.** Alles parametrisiert.
* **Inline-Auslieferung nur über eine Positivliste** harmloser Typen, mit
`sandbox`, und `allow-scripts` ausschließlich für PDF.
* **Ratenbegrenzung an jeder unangemeldeten Tür**, die eines braucht: Anmeldung
(20/min), WebDAV, CalDAV, Freigabelinks (je 10 Fehlversuche / 15 Minuten).
Die übrigen offenen Wege (`/health`, `/branding`, `/manifest.webmanifest`,
`/api/v1/public/config`) geben nichts preis.
* **Kein `dangerouslySetInnerHTML`, kein `eval`** im Frontend.
* **Sechs Selbsttest-Suiten, 220 Prüfungen** gegen die laufende Instanz, dazu
37 Frontend-Tests und die Go-Tests.
---
## 5 · Kleinere offene Enden
* **`internal/groups` ist ein leerer Stub** (0 Zeilen). Gruppenordner liegen in
`files/groupfolder.go`. Entweder das Verzeichnis weg oder der Code dorthin —
ein leeres Paket im Baum ist eine Frage, die jeder einmal stellt.
* **`favicon_storage_key`** steht als leere Zeile in den Einstellungen; die
Funktion heißt seit dem App-Symbol anders. Schadet nichts, wird ignoriert.
* **Die Fehlversuchssperren liegen im Arbeitsspeicher.** Ein Neustart des
Containers hebt jede Sperre auf. Für den Hausgebrauch vertretbar; wer es
anders will, braucht eine Tabelle.
* **Frontend ohne Komponententests.** `vitest` läuft jetzt (37 Tests), aber nur
über reine Funktionen — für DOM-Tests fehlt `jsdom` in den devDependencies.
---
## Reihenfolge, die ich vorschlagen würde
1. ~~`recover` in die Hintergrund-Goroutinen~~ ✅ 12.09.2026
2. ~~Prüfsummen-Abweichung melden~~ ✅ 12.09.2026 – und dabei fiel auf, dass
**alle** kontextgebundenen Logzeilen ins Nichts gingen (2b).
3. **Tests für `documents`** (Punkt 3). Das Paket, dessen stilles Versagen am
teuersten ist: Die Suche findet dann einfach nichts, und das sieht aus wie
„ist nicht drin".