internal/tags hatte null Tests, und die Rechtepruefung steckt dort nicht in einer if-Zeile, sondern in WHERE-Klauseln. Ein Tippfehler darin faellt beim Lesen nicht auf: Die Abfrage laeuft, sie liefert nur zu viel. Fuenf Tests auf dieselbe Frage -- Liste, Umbenennen, Loeschen, Anhaengen an die eigene Datei, und vor allem das Auflisten der **Dateien** unter einem Etikett. Der letzte Weg gibt Dateinamen, Ordner und Groessen heraus; wer dort eine fremde Etikett-ID erraet, saehe einen Ausschnitt aus fremden Unterlagen. Dazu Gruppen: Mitglied ja, Nichtmitglied nein, und wer die Gruppe verlaesst, verliert den Zugriff im selben Moment. Gegen kaputten Code gegengeprueft: Eigentuemerpruefung aus FilesWithTag entfernt -> "Bert sieht 1 fremde Dateien: Gehaltsabrechnung.pdf". Pruefung in SetFieldsForFile ausgehebelt -> zwei Tests rot. **Ein Verdacht erwies sich als falsch, und das ist der nuetzlichere Ausgang:** List bietet Gruppen-Etiketten an, SetFieldsForFile vergleicht gegen den Prinzipal der Datei -- sah nach einem Widerspruch aus. Ist keiner; die Oberflaeche filtert bereits (TagDialog.tsx:69). Und die Ablehnung ist nicht Foermlichkeit, sondern die tragende Absicherung: FilesWithTag prueft, ob man das *Etikett* sehen darf, nicht die *Datei*. Das ist nur deshalb harmlos, weil ein Etikett stets denselben Prinzipal hat wie die Dateien darunter. Der Test haelt jetzt diesen Zusammenhang fest, nicht bloss eine Fehlermeldung. Nebenbei: tags hat zwei Fehlermarken fuer dieselbe Aussage (ErrNotFound und ErrCorrespondentNotFound). Kein Fehler, aber eine Stolperstelle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
265 lines
12 KiB
Markdown
265 lines
12 KiB
Markdown
# 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.
|
||
|
||
**Teilweise erledigt am 12.09.2026:** `internal/documents` hat jetzt acht
|
||
Tests — gezielt auf die drei Zustände, in denen die Warteschlange
|
||
hängenbleiben kann, ohne dass es auffällt (Details in
|
||
[TESTING.md](TESTING.md)). Dazu `scripts/db-tests.sh`, damit sie nicht wie
|
||
`vitest` zur Dekoration werden: Sie überspringen sich ohne `TEST_DSN` und
|
||
liefen sonst nie.
|
||
|
||
Gegengeprüft an absichtlich kaputtem Code — und **der zweite Versuch blieb
|
||
grün**: `maxAttempts` von 3 auf 9999 gesetzt, alle Tests weiter in Ordnung,
|
||
nur die Laufzeit stieg von 0,2 auf 27 Sekunden. Die Tests schleiften über die
|
||
Konstante und setzten die Zahl voraus, statt sie zu prüfen. Dagegen steht jetzt
|
||
ein eigener Test.
|
||
|
||
**`internal/tags` ebenfalls erledigt am 12.09.2026.** Fünf Tests, alle auf
|
||
dieselbe Frage: Bleibt ein fremdes Etikett fremd – in der Liste, beim
|
||
Umbenennen, beim Löschen, beim Anhängen an die eigene Datei, und vor allem beim
|
||
**Auflisten der Dateien darunter**. Der letzte Weg gibt Dateinamen, Ordner und
|
||
Größen heraus; wer dort eine fremde Etikett-ID errät, sähe einen Ausschnitt aus
|
||
fremden Unterlagen.
|
||
|
||
Auch hier gegen kaputten Code gegengeprüft: Eigentümerprüfung aus
|
||
`FilesWithTag` entfernt → *„Bert sieht 1 fremde Dateien: Gehaltsabrechnung.pdf"*.
|
||
Prüfung in `SetFieldsForFile` ausgehebelt → zwei Tests rot.
|
||
|
||
**Ein Verdacht erwies sich dabei als falsch, und das ist der nützlichere
|
||
Ausgang:** `List` bietet Gruppen-Etiketten an, `SetFieldsForFile` vergleicht
|
||
gegen den Prinzipal der *Datei* – das sah nach einem Widerspruch aus, bei dem
|
||
die Oberfläche etwas anbietet, was der Server ablehnt. Ist keiner: Die
|
||
Oberfläche filtert bereits (`TagDialog.tsx:69`). Und die Ablehnung ist nicht
|
||
Förmlichkeit, sondern die tragende Absicherung – `FilesWithTag` prüft, ob man
|
||
das *Etikett* sehen darf, **nicht** die *Datei*. Das ist nur deshalb harmlos,
|
||
weil ein Etikett stets denselben Prinzipal hat wie die Dateien darunter. Fiele
|
||
die Prüfung weg, hinge an einem Gruppen-Etikett eine private Datei, und jedes
|
||
Gruppenmitglied sähe ihren Namen. Der Test hält jetzt diesen Zusammenhang fest,
|
||
nicht bloß eine Fehlermeldung.
|
||
|
||
Nebenbei: `internal/tags` hat **zwei** Fehlermarken für dieselbe Aussage –
|
||
`ErrNotFound` und `ErrCorrespondentNotFound`. Kein Fehler, aber eine
|
||
Stolperstelle; jetzt wenigstens festgehalten.
|
||
|
||
**Offen bleiben** `internal/sharing`, `internal/rescan` (beide mit
|
||
Python-Selbsttest), `internal/audit` und `internal/thumbnails`.
|
||
|
||
---
|
||
|
||
## 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`~~ ✅ und ~~`tags`~~ ✅ 12.09.2026.
|