Files
Aether/BEFUNDE.md
T
marcandClaude Opus 5 6042f6f2f6 test(tags): bleibt ein fremdes Etikett fremd?
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>
2026-09-12 23:04:04 +02:00

12 KiB
Raw Blame History

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 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:

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:

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). 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.