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>
This commit is contained in:
marcandClaude Opus 5 committed 2026-09-12 23:04:04 +02:00
1 parent 76176a45bc
commit 6042f6f2f6
3 files changed
+378 -4

No files matched your search

+30 -4
View File
@@ -187,8 +187,35 @@ 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.
**Offen bleibt `internal/tags`:** Etiketten sind je Konto, und dass ein fremdes
Etikett fremd bleibt, prüft bis heute nichts nach.
**`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`.
---
@@ -234,5 +261,4 @@ gibt:
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`~~ ✅ 12.09.2026. Offen: `internal/tags` — die
Rechteprüfung dort ist ungeprüft.
3. ~~Tests für `documents`~~ ✅ und ~~`tags`~~ ✅ 12.09.2026.
+29
View File
@@ -269,6 +269,29 @@ es auffällt:
Dazu: gelöschte Dateien werden nicht mehr genommen, ein erneuter Upload gibt
eine neue Chance, und `Complete` schreibt Text und Auftragsende zusammen.
### Etiketten und Absender: bleibt Fremdes fremd?
./scripts/db-tests.sh ./internal/tags/
Die Rechteprüfung steckt hier nicht in einer `if`-Zeile, sondern in
WHERE-Klauseln. Ein Tippfehler darin fällt beim Lesen nicht auf: Die Abfrage
läuft, sie liefert nur zu viel.
Fünf Tests auf dieselbe Frage – Liste, Umbenennen, Löschen, Anhängen an die
eigene Datei, und vor allem das **Auflisten der Dateien** unter einem Etikett.
Der letzte Weg gibt Dateinamen, Ordner und Größen heraus.
Dazu die Gruppen: Ein Mitglied sieht das Gruppen-Etikett, ein Nichtmitglied
nicht, und wer die Gruppe verlässt, verliert den Zugriff im selben Moment.
**Eine Absicherung, die wie eine Förmlichkeit aussieht:** Ein Gruppen-Etikett
darf nicht an eine persönliche Datei. Der Grund steht nicht im Code, wo die
Prüfung sitzt, sondern zwei Dateien weiter: `FilesWithTag` prüft, ob der
Anfragende das *Etikett* sehen darf – **nicht**, ob er die *Datei* sehen darf.
Harmlos ist das nur, 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 bekäme ihren Namen zu sehen.
### Was der Mutationsversuch gezeigt hat
Die Tests wurden gegen **absichtlich kaputten Code** gegengeprüft – sonst weiß
@@ -283,6 +306,12 @@ Genau so verschwindet eine Entscheidung unbemerkt aus einer Anwendung. Dagegen
steht jetzt ein Test, der die Zahl selbst festhält und sagt, wo die Begründung
hingehört, falls sie sich ändert.
Bei den Etiketten dasselbe Vorgehen, mit klarerem Ausgang: Eigentümerprüfung
aus `FilesWithTag` entfernt → *„Bert sieht 1 fremde Dateien:
Gehaltsabrechnung.pdf"*. Prüfung in `SetFieldsForFile` ausgehebelt → zwei Tests
rot. Beide Absicherungen sind also wirklich abgedeckt und nicht bloß
mitgelaufen.
## Notfall: Migration hängt fest ("Dirty database version")
Betrifft jede künftige Migration, nicht nur eine bestimmte. Nachgestellt und
+319
View File
@@ -0,0 +1,319 @@
package tags
import (
"context"
"errors"
"os"
"testing"
"github.com/google/uuid"
"github.com/jackc/pgx/v5/pgxpool"
)
// Etiketten und Absender gehören einem Konto oder einer Gruppe. Dass ein
// fremdes Etikett fremd **bleibt**, prüfte bis zum 12.09.2026 nichts nach –
// und die Prüfung steckt hier nicht in einer if-Zeile, sondern in
// WHERE-Klauseln. Ein Tippfehler darin fällt beim Lesen nicht auf: Die Abfrage
// läuft, sie liefert nur zu viel.
//
// Der teuerste Fall ist FilesWithTag: Es gibt **Dateinamen und Ordner**
// heraus. Wer dort eine fremde Etikett-ID errät, sähe einen Ausschnitt aus
// fremden Unterlagen.
//
// Läuft nur mit TEST_DSN. Einfachster Weg: ./scripts/db-tests.sh ./internal/tags/
func testStore(t *testing.T) (*Store, *pgxpool.Pool, context.Context) {
t.Helper()
dsn := os.Getenv("TEST_DSN")
if dsn == "" {
t.Skip("TEST_DSN nicht gesetzt")
}
ctx := context.Background()
pool, err := pgxpool.New(ctx, dsn)
if err != nil {
t.Fatalf("verbinden: %v", err)
}
t.Cleanup(pool.Close)
return NewStore(pool), pool, ctx
}
func konto(t *testing.T, pool *pgxpool.Pool, ctx context.Context, name string) uuid.UUID {
t.Helper()
var id uuid.UUID
err := pool.QueryRow(ctx,
`INSERT INTO users (username, display_name, password, is_admin)
VALUES ($1, $1, 'x', false) RETURNING id`, name+"-"+uuid.NewString()[:8],
).Scan(&id)
if err != nil {
t.Fatalf("konto %s: %v", name, err)
}
t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM users WHERE id = $1`, id) })
return id
}
func datei(t *testing.T, pool *pgxpool.Pool, ctx context.Context, owner uuid.UUID, name string) uuid.UUID {
t.Helper()
var id uuid.UUID
err := pool.QueryRow(ctx,
`INSERT INTO files (owner_id, folder_id, name, mime_type, size, checksum, storage_key)
VALUES ($1, NULL, $2, 'application/pdf', 1, 'x', $3) RETURNING id`,
owner, name, "k/"+uuid.NewString(),
).Scan(&id)
if err != nil {
t.Fatalf("datei: %v", err)
}
return id
}
// ── Ein fremdes Etikett bleibt fremd ────────────────────────────────────────
func TestFremdesEtikettBleibtFremd(t *testing.T) {
s, pool, ctx := testStore(t)
a := konto(t, pool, ctx, "anna")
b := konto(t, pool, ctx, "bert")
annas, err := s.Create(ctx, a, nil, "Steuer 2026", "#ff0000", "")
if err != nil {
t.Fatalf("anlegen: %v", err)
}
t.Run("taucht nicht in Berts Liste auf", func(t *testing.T) {
liste, err := s.List(ctx, b)
if err != nil {
t.Fatalf("list: %v", err)
}
for _, tg := range liste {
if tg.ID == annas.ID {
t.Fatal("Bert sieht Annas Etikett")
}
}
})
t.Run("Bert kann es nicht umbenennen", func(t *testing.T) {
if _, err := s.Update(ctx, b, annas.ID, "Uebernommen", "#00ff00", ""); !errors.Is(err, ErrNotFound) {
t.Fatalf("Update lieferte %v, erwartet ErrNotFound", err)
}
// Und der Name steht wirklich noch da – der Fehlercode allein sagt
// nichts darüber, ob nebenbei geschrieben wurde.
var name string
_ = pool.QueryRow(ctx, `SELECT name FROM tags WHERE id = $1`, annas.ID).Scan(&name)
if name != "Steuer 2026" {
t.Fatalf("Name ist jetzt %q", name)
}
})
t.Run("Bert kann es nicht löschen", func(t *testing.T) {
if err := s.Delete(ctx, b, annas.ID); !errors.Is(err, ErrNotFound) {
t.Fatalf("Delete lieferte %v, erwartet ErrNotFound", err)
}
var da bool
_ = pool.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM tags WHERE id = $1)`, annas.ID).Scan(&da)
if !da {
t.Fatal("Annas Etikett ist weg")
}
})
// Der teuerste Fall: Hier kämen Dateinamen und Ordner heraus.
t.Run("Bert sieht die Dateien darunter nicht", func(t *testing.T) {
f := datei(t, pool, ctx, a, "Gehaltsabrechnung.pdf")
if err := s.SetForFile(ctx, f, a, a, []uuid.UUID{annas.ID}, nil); err != nil {
t.Fatalf("etikettieren: %v", err)
}
// Anna sieht sie – sonst prüfte der nächste Teil eine leere Liste.
meine, err := s.FilesWithTag(ctx, a, annas.ID)
if err != nil || len(meine) != 1 {
t.Fatalf("Anna sieht ihre eigene Datei nicht: %v, %d Treffer", err, len(meine))
}
fremde, err := s.FilesWithTag(ctx, b, annas.ID)
if err != nil {
t.Fatalf("FilesWithTag: %v", err)
}
if len(fremde) != 0 {
t.Fatalf("Bert sieht %d fremde Dateien: %v", len(fremde), fremde[0].Name)
}
})
t.Run("Bert kann es nicht an seine eigene Datei hängen", func(t *testing.T) {
f := datei(t, pool, ctx, b, "Berts Datei.pdf")
err := s.SetForFile(ctx, f, b, b, []uuid.UUID{annas.ID}, nil)
if !errors.Is(err, ErrWrongOwner) {
t.Fatalf("SetForFile lieferte %v, erwartet ErrWrongOwner", err)
}
var n int
_ = pool.QueryRow(ctx, `SELECT COUNT(*) FROM file_tags WHERE file_id = $1`, f).Scan(&n)
if n != 0 {
t.Fatalf("%d Zuordnungen entstanden trotzdem", n)
}
})
}
// ── Gruppen ─────────────────────────────────────────────────────────────────
func gruppeMit(t *testing.T, pool *pgxpool.Pool, ctx context.Context, mitglieder ...uuid.UUID) uuid.UUID {
t.Helper()
var g uuid.UUID
err := pool.QueryRow(ctx,
`INSERT INTO groups (name) VALUES ($1) RETURNING id`, "probe-"+uuid.NewString()[:8],
).Scan(&g)
if err != nil {
t.Fatalf("gruppe: %v", err)
}
t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM groups WHERE id = $1`, g) })
for _, m := range mitglieder {
if _, err := pool.Exec(ctx,
`INSERT INTO group_members (group_id, user_id) VALUES ($1, $2)`, g, m); err != nil {
t.Fatalf("mitglied: %v", err)
}
}
return g
}
func TestGruppenEtikettSiehtNurDieGruppe(t *testing.T) {
s, pool, ctx := testStore(t)
drin := konto(t, pool, ctx, "drin")
draussen := konto(t, pool, ctx, "draussen")
g := gruppeMit(t, pool, ctx, drin)
tag, err := s.Create(ctx, drin, &g, "Buchhaltung", "#0000ff", "")
if err != nil {
t.Fatalf("anlegen: %v", err)
}
sieht := func(user uuid.UUID) bool {
liste, err := s.List(ctx, user)
if err != nil {
t.Fatalf("list: %v", err)
}
for _, x := range liste {
if x.ID == tag.ID {
return true
}
}
return false
}
if !sieht(drin) {
t.Fatal("das Mitglied sieht das Gruppen-Etikett nicht")
}
if sieht(draussen) {
t.Fatal("ein Nichtmitglied sieht das Gruppen-Etikett")
}
// Mitgliedschaft ersetzt die Freigabe: Wer die Gruppe verlässt, verliert
// den Zugriff im selben Moment.
if _, err := pool.Exec(ctx,
`DELETE FROM group_members WHERE group_id = $1 AND user_id = $2`, g, drin); err != nil {
t.Fatalf("austragen: %v", err)
}
if sieht(drin) {
t.Fatal("nach dem Austritt weiterhin sichtbar")
}
if _, err := s.Update(ctx, drin, tag.ID, "Umbenannt", "#111111", ""); !errors.Is(err, ErrNotFound) {
t.Fatalf("nach dem Austritt noch änderbar: %v", err)
}
}
// Ein Gruppen-Etikett gehört **nicht** an eine persönliche Datei – und das ist
// keine Förmlichkeit, sondern die Stelle, an der die Rechte hängen.
//
// Der Anlass war ein Verdacht, der sich als falsch erwies: `List` bietet dem
// Nutzer Gruppen-Etiketten mit an, `SetFieldsForFile` vergleicht aber gegen
// den *Prinzipal der Datei*. Sieht nach Widerspruch aus. Ist keiner – die
// Oberfläche filtert bereits (`TagDialog.tsx`: bei einer persönlichen Datei
// nur Etiketten ohne group_id).
//
// **Warum die Ablehnung bleiben muss:** `FilesWithTag` prüft, ob der
// Anfragende das *Etikett* sehen darf – aber **nicht**, ob er die *Datei*
// sehen darf. Das ist nur deshalb harmlos, weil ein Etikett stets denselben
// Prinzipal hat wie die Dateien darunter: ein persönliches nur an eigenen
// Dateien, ein Gruppen-Etikett nur an Gruppendateien, die alle Mitglieder
// ohnehin sehen.
//
// Fiele diese Prüfung weg, hinge an einem Gruppen-Etikett eine private Datei –
// und jedes Gruppenmitglied bekäme über die Etikettenansicht ihren Namen,
// Ordner und ihre Größe zu sehen. Deshalb steht dieser Test hier: Er hält
// nicht eine Fehlermeldung fest, sondern eine Absicherung.
func TestGruppenEtikettGehoertNichtAnEinePersoenlicheDatei(t *testing.T) {
s, pool, ctx := testStore(t)
u := konto(t, pool, ctx, "mitglied")
g := gruppeMit(t, pool, ctx, u)
tag, err := s.Create(ctx, u, &g, "Gemeinsam", "#00ffff", "")
if err != nil {
t.Fatalf("anlegen: %v", err)
}
f := datei(t, pool, ctx, u, "Privat.pdf")
// Prinzipal der Datei ist der Nutzer, nicht die Gruppe.
if err := s.SetForFile(ctx, f, u, u, []uuid.UUID{tag.ID}, nil); !errors.Is(err, ErrWrongOwner) {
t.Fatalf("SetForFile lieferte %v, erwartet ErrWrongOwner – ohne diese "+
"Prüfung sähe jedes Gruppenmitglied diese private Datei", err)
}
var n int
_ = pool.QueryRow(ctx, `SELECT COUNT(*) FROM file_tags WHERE file_id = $1`, f).Scan(&n)
if n != 0 {
t.Fatalf("%d Zuordnungen entstanden trotzdem", n)
}
// Die Gegenprobe: An einer **Gruppendatei** geht dasselbe Etikett.
var gf uuid.UUID
err = pool.QueryRow(ctx,
`INSERT INTO files (owner_id, group_id, folder_id, name, mime_type, size, checksum, storage_key)
VALUES (NULL, $1, NULL, 'Gemeinsam.pdf', 'application/pdf', 1, 'x', $2) RETURNING id`,
g, "k/"+uuid.NewString()).Scan(&gf)
if err != nil {
t.Fatalf("gruppendatei: %v", err)
}
if err := s.SetForFile(ctx, gf, g, u, []uuid.UUID{tag.ID}, nil); err != nil {
t.Fatalf("an einer Gruppendatei muss es gehen: %v", err)
}
}
// ── Absender ────────────────────────────────────────────────────────────────
func TestFremderAbsenderBleibtFremd(t *testing.T) {
s, pool, ctx := testStore(t)
a := konto(t, pool, ctx, "anna")
b := konto(t, pool, ctx, "bert")
k, err := s.CreateCorrespondent(ctx, a, nil, "Finanzamt", "")
if err != nil {
t.Fatalf("anlegen: %v", err)
}
liste, err := s.ListCorrespondents(ctx, b)
if err != nil {
t.Fatalf("list: %v", err)
}
for _, x := range liste {
if x.ID == k.ID {
t.Fatal("Bert sieht Annas Absender")
}
}
// Absender haben ihre eigene Fehlermarke – ErrCorrespondentNotFound, nicht
// ErrNotFound. Zwei Marken für dieselbe Aussage in einem Paket sind eine
// Stolperstelle; hier festgehalten, damit sie wenigstens nicht unbemerkt
// die Seite wechselt.
if _, err := s.UpdateCorrespondent(ctx, b, k.ID, "Uebernommen", ""); !errors.Is(err, ErrCorrespondentNotFound) {
t.Fatalf("Update lieferte %v, erwartet ErrCorrespondentNotFound", err)
}
if err := s.DeleteCorrespondent(ctx, b, k.ID); !errors.Is(err, ErrCorrespondentNotFound) {
t.Fatalf("Delete lieferte %v, erwartet ErrCorrespondentNotFound", err)
}
// Auch hier: Der Weg, der Dateinamen herausgibt.
f := datei(t, pool, ctx, a, "Bescheid.pdf")
if err := s.SetFieldsForFile(ctx, f, a, a, FileFields{CorrespondentID: &k.ID}); err != nil {
t.Fatalf("zuordnen: %v", err)
}
meine, err := s.FilesWithCorrespondent(ctx, a, k.ID)
if err != nil || len(meine) != 1 {
t.Fatalf("Anna sieht ihre eigene Datei nicht: %v, %d", err, len(meine))
}
fremde, err := s.FilesWithCorrespondent(ctx, b, k.ID)
if err != nil {
t.Fatalf("FilesWithCorrespondent: %v", err)
}
if len(fremde) != 0 {
t.Fatalf("Bert sieht %d fremde Dateien", len(fremde))
}
}