feat(mail): SRC-11 geschlossener FacetField-Typ statt Whitelist-Liste
fields.go: neuer Typ FacetField mit vier geschlossenen Konstanten (FacetFieldSender/Mailbox/AttachmentType/Tag). IsValid() entscheidet über ein erschöpfendes switch/case statt eine []string-Liste zu durchsuchen — genau der aus known-issues-archivmail.md #12 und known-issues-archivdms.md #10 bekannte Fehler (dynamische Tabellen-/ Feldnamen nur durch eine fragile Whitelist-Funktion abgesichert) wird damit strukturell vermieden: ein vergessener Listeneintrag kann nichts mehr durchlassen, weil es keine durchsuchte Liste mehr gibt. ParseFacetField ist die einzige vorgesehene Konstruktionsstelle für FacetField aus einer externen Zeichenkette. facets.go: FacetFilter.Field ist jetzt FacetField statt string, buildFilteredMust prüft f.Field.IsValid() statt Listenmitgliedschaft (isFacetField entfernt, es gibt keine Liste mehr, die die Entscheidung trifft). Alle Pflichtprüfungen mit echten Nachweisen: unbekannte/erfundene Facettenfelder werden abgelehnt, alle vier realen Facettenfelder funktionieren weiterhin, ein FacetField-Wert per direkter Typkonvertierung (nicht über ParseFacetField) wird trotzdem zuverlässig abgelehnt (Akzeptanzkriterium 2: Whitelist ist nicht die einzige Absicherung), automatisiertes Code-Review bestätigt kein fmt.Sprintf in facets.go/fields.go. Entscheidung dokumentiert: Mail-eigene Implementierung, keine geteilte Utility mit dem DMS-Board (Prüfprotokoll). Keine Regression, insbesondere mail/internal/savedsearch (Konsument von FacetFilter) unverändert grün — go build/go vet/golangci-lint clean, gesamtes Mail-Modul regressionsfrei getestet.
This commit is contained in:
sysops
@@ -14,11 +14,13 @@ import (
|
||||
)
|
||||
|
||||
// FacetFilter schränkt Suche/Facettenberechnung auf einen bereits
|
||||
// gewählten Facettenwert ein. Field MUSS aus FacetFields stammen —
|
||||
// Facets liefert einen Fehler bei jedem anderen Wert (verhindert einen
|
||||
// beliebigen, vom Aufrufer bestimmten Feldnamen in der Anfrage).
|
||||
// gewählten Facettenwert ein. Field ist der geschlossene FacetField-Typ
|
||||
// (SRC-11) — buildFilteredMust prüft zusätzlich FacetField.IsValid(),
|
||||
// sodass selbst ein über json.Unmarshal aus der Datenbank
|
||||
// rekonstruierter, nicht mehr gültiger Wert (z. B. nach Entfernen eines
|
||||
// Feldes) abgelehnt wird statt stillschweigend durchzulaufen.
|
||||
type FacetFilter struct {
|
||||
Field string
|
||||
Field FacetField
|
||||
Value string
|
||||
}
|
||||
|
||||
@@ -70,19 +72,12 @@ func dateRangeBoundaries(now time.Time) []dateRangeBoundary {
|
||||
}
|
||||
}
|
||||
|
||||
func isFacetField(field string) bool {
|
||||
for _, f := range FacetFields {
|
||||
if f == field {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// buildFilteredMust baut die gemeinsame bool.must-Liste für Facets und
|
||||
// SearchWithFilters: Tenant-Filter zwingend, optionaler Suchtext, dann je
|
||||
// Filter eine zusätzliche equals-Klausel (UND-Verknüpfung) — einzige
|
||||
// Stelle, an der Filter-Feldnamen gegen FacetFields geprüft werden.
|
||||
// Stelle, an der Filter-Feldnamen geprüft werden, über das geschlossene
|
||||
// FacetField.IsValid() (SRC-11 Akzeptanzkriterium 2), nicht über eine
|
||||
// durchsuchbare Liste.
|
||||
func buildFilteredMust(tenantSlug, queryText string, filters []FacetFilter) ([]map[string]any, error) {
|
||||
must := []map[string]any{
|
||||
{"equals": map[string]any{FieldTenantSlug: tenantSlug}},
|
||||
@@ -91,10 +86,10 @@ func buildFilteredMust(tenantSlug, queryText string, filters []FacetFilter) ([]m
|
||||
must = append(must, map[string]any{"query_string": queryText})
|
||||
}
|
||||
for _, f := range filters {
|
||||
if !isFacetField(f.Field) {
|
||||
if !f.Field.IsValid() {
|
||||
return nil, fmt.Errorf("search: unbekanntes facettenfeld %q", f.Field)
|
||||
}
|
||||
must = append(must, map[string]any{"equals": map[string]any{f.Field: f.Value}})
|
||||
must = append(must, map[string]any{"equals": map[string]any{string(f.Field): f.Value}})
|
||||
}
|
||||
return must, nil
|
||||
}
|
||||
@@ -162,7 +157,7 @@ func (c *Client) Facets(ctx context.Context, tenantSlug, queryText string, filte
|
||||
|
||||
aggs := map[string]any{}
|
||||
for _, field := range FacetFields {
|
||||
aggs[field] = map[string]any{"terms": map[string]any{"field": field, "size": 100}}
|
||||
aggs[string(field)] = map[string]any{"terms": map[string]any{"field": string(field), "size": 100}}
|
||||
}
|
||||
boundaries := dateRangeBoundaries(time.Now())
|
||||
ranges := make([]map[string]any, 0, len(boundaries))
|
||||
@@ -208,7 +203,7 @@ func (c *Client) Facets(ctx context.Context, tenantSlug, queryText string, filte
|
||||
|
||||
result := FacetResult{Values: make(map[string][]FacetValue, len(FacetFields))}
|
||||
for _, field := range FacetFields {
|
||||
bucket := parsed.Aggregations[field]
|
||||
bucket := parsed.Aggregations[string(field)]
|
||||
values := make([]FacetValue, 0, len(bucket.Buckets))
|
||||
for _, b := range bucket.Buckets {
|
||||
if b.Key == "" {
|
||||
@@ -216,7 +211,7 @@ func (c *Client) Facets(ctx context.Context, tenantSlug, queryText string, filte
|
||||
}
|
||||
values = append(values, FacetValue{Value: b.Key, Count: b.DocCount})
|
||||
}
|
||||
result.Values[field] = values
|
||||
result.Values[string(field)] = values
|
||||
}
|
||||
|
||||
sentAtBucket := parsed.Aggregations["sent_at"]
|
||||
|
||||
@@ -35,12 +35,52 @@ const (
|
||||
FieldOCRConfidence = "ocr_confidence"
|
||||
)
|
||||
|
||||
// FacetField ist ein geschlossener Typ für die vier zulässigen
|
||||
// Facetten-/Filterdimensionen (SRC-11, Akzeptanzkriterium 2): die
|
||||
// Zulässigkeitsprüfung in facets.go läuft über ein erschöpfendes
|
||||
// switch/case auf diesem Typ, NICHT über das Durchsuchen einer Liste —
|
||||
// selbst ein vergessener Eintrag in einer Whitelist-Liste könnte dort
|
||||
// nichts mehr durchlassen, weil keine solche Liste mehr die Entscheidung
|
||||
// trifft. FacetFields (unten) ist nur noch eine abgeleitete
|
||||
// Aufzählungshilfe für Iteration, keine Prüfgrundlage.
|
||||
type FacetField string
|
||||
|
||||
const (
|
||||
FacetFieldSender FacetField = FacetField(FieldSender)
|
||||
FacetFieldMailbox FacetField = FacetField(FieldMailbox)
|
||||
FacetFieldAttachmentType FacetField = FacetField(FieldAttachmentType)
|
||||
FacetFieldTag FacetField = FacetField(FieldTag)
|
||||
)
|
||||
|
||||
// IsValid entscheidet über Zulässigkeit als Facetten-/Filterfeld über
|
||||
// ein geschlossenes switch/case (Akzeptanzkriterium 2) statt eine Liste
|
||||
// zu durchsuchen.
|
||||
func (f FacetField) IsValid() bool {
|
||||
switch f {
|
||||
case FacetFieldSender, FacetFieldMailbox, FacetFieldAttachmentType, FacetFieldTag:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// ParseFacetField wandelt eine externe Zeichenkette (z. B. aus einem
|
||||
// HTTP-Query-Parameter) in ein FacetField um — liefert false bei jedem
|
||||
// Wert, der nicht exakt einer der geschlossenen Konstanten entspricht.
|
||||
// Einzige vorgesehene Stelle, an der ein Client-Feldname überhaupt zu
|
||||
// einem FacetField werden kann.
|
||||
func ParseFacetField(raw string) (FacetField, bool) {
|
||||
f := FacetField(raw)
|
||||
return f, f.IsValid()
|
||||
}
|
||||
|
||||
// FacetFields sind die je Kachel unterstützten Filterdimensionen
|
||||
// (Akzeptanzkriterium 1: Absender, Postfach, Anhangstyp, Tag — Zeitraum
|
||||
// läuft separat über FieldSentAt als Bereichsfacette, siehe facets.go).
|
||||
// Statische Liste — Aufrufer können ausschließlich diese Feldnamen als
|
||||
// Facetten-/Filterdimension angeben, kein beliebiger Client-Feldname.
|
||||
var FacetFields = []string{FieldSender, FieldMailbox, FieldAttachmentType, FieldTag}
|
||||
// Nur zur Iteration gedacht (z. B. "berechne alle Facetten") — die
|
||||
// Zulässigkeitsprüfung selbst läuft über FacetField.IsValid(), nicht
|
||||
// über Mitgliedschaft in dieser Liste.
|
||||
var FacetFields = []FacetField{FacetFieldSender, FacetFieldMailbox, FacetFieldAttachmentType, FacetFieldTag}
|
||||
|
||||
// DocumentID berechnet deterministisch die Manticore-Dokument-ID aus
|
||||
// Mandant und Message-ID (FNV-1a, 64 Bit). Deterministisch statt einer
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
// SRC-11: Feld-Whitelist-Query-Builder für Suchindex-Zugriff. Reine
|
||||
// Unit-Tests (kein Manticore nötig) — buildFilteredMust und FacetField
|
||||
// sind pure Funktionen/Typen.
|
||||
package search
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestBuildFilteredMust_RejectsUnknownField ist die geforderte
|
||||
// Pflichtprüfung 1 (SRC-11): Versuch, ein nicht in der Whitelist
|
||||
// enthaltenes Feld anzufragen, wird abgewiesen statt stillschweigend
|
||||
// ignoriert.
|
||||
func TestBuildFilteredMust_RejectsUnknownField(t *testing.T) {
|
||||
// FacetField(...) simuliert genau den Fall, den Akzeptanzkriterium 2
|
||||
// verlangt: ein Wert, der NICHT über die vorgesehene
|
||||
// ParseFacetField-Konstruktion entstanden ist (z. B. aus einem
|
||||
// veralteten Datenbankeintrag nach Entfernen eines Feldes) — muss
|
||||
// trotzdem abgelehnt werden.
|
||||
unknown := FacetField("tenant_slug") // existiert als Suchfeld, ist aber KEIN Facettenfeld
|
||||
_, err := buildFilteredMust("mandant-x", "", []FacetFilter{{Field: unknown, Value: "x"}})
|
||||
if err == nil {
|
||||
t.Fatalf("erwartete ablehnung für unbekanntes facettenfeld %q, bekam keinen fehler", unknown)
|
||||
}
|
||||
if !strings.Contains(err.Error(), string(unknown)) {
|
||||
t.Fatalf("fehlermeldung sollte das abgelehnte feld nennen, habe: %v", err)
|
||||
}
|
||||
|
||||
// Frei erfundenes Feld, das nirgendwo im Schema existiert.
|
||||
madeUp := FacetField("'; DROP TABLE mail_documents; --")
|
||||
_, err = buildFilteredMust("mandant-x", "", []FacetFilter{{Field: madeUp, Value: "x"}})
|
||||
if err == nil {
|
||||
t.Fatalf("erwartete ablehnung für frei erfundenes facettenfeld, bekam keinen fehler")
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildFilteredMust_AcceptsAllWhitelistedFields stellt sicher, dass
|
||||
// alle vier vorgesehenen Facettenfelder tatsächlich funktionieren (keine
|
||||
// versehentlich zu strenge Prüfung).
|
||||
func TestBuildFilteredMust_AcceptsAllWhitelistedFields(t *testing.T) {
|
||||
for _, field := range FacetFields {
|
||||
_, err := buildFilteredMust("mandant-x", "", []FacetFilter{{Field: field, Value: "x"}})
|
||||
if err != nil {
|
||||
t.Fatalf("feld %q hätte akzeptiert werden müssen: %v", field, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestFacetField_ClosedSetEvenViaDirectTypeConversion ist die geforderte
|
||||
// Pflichtprüfung/Akzeptanzkriterium 2: die Whitelist ist nicht die
|
||||
// einzige Absicherung. Selbst ein FacetField-Wert, der NICHT über
|
||||
// ParseFacetField entstanden ist (direkte Typkonvertierung, z. B. durch
|
||||
// künftigen Code, der die vorgesehene Konstruktion umgeht), wird von
|
||||
// IsValid() zuverlässig abgelehnt — die Prüfung hängt an einem
|
||||
// erschöpfenden switch/case auf den vier Konstanten, nicht an einer
|
||||
// durchsuchbaren Liste, die vergessen werden könnte.
|
||||
func TestFacetField_ClosedSetEvenViaDirectTypeConversion(t *testing.T) {
|
||||
valid := []FacetField{FacetFieldSender, FacetFieldMailbox, FacetFieldAttachmentType, FacetFieldTag}
|
||||
for _, f := range valid {
|
||||
if !f.IsValid() {
|
||||
t.Fatalf("erwartete gültiges feld %q als gültig", f)
|
||||
}
|
||||
}
|
||||
|
||||
invalid := []FacetField{
|
||||
FacetField(FieldTenantSlug), // reales Suchfeld, aber keine Facette
|
||||
FacetField(FieldBody),
|
||||
FacetField("subject; --"),
|
||||
FacetField(""),
|
||||
}
|
||||
for _, f := range invalid {
|
||||
if f.IsValid() {
|
||||
t.Fatalf("feld %q hätte als ungültig erkannt werden müssen", f)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestParseFacetField_OnlyAcceptsKnownStrings deckt die einzige
|
||||
// vorgesehene Konstruktionsstelle für FacetField aus einer externen
|
||||
// Zeichenkette ab.
|
||||
func TestParseFacetField_OnlyAcceptsKnownStrings(t *testing.T) {
|
||||
if _, ok := ParseFacetField("sender"); !ok {
|
||||
t.Fatalf("'sender' hätte als gültiges facettenfeld erkannt werden müssen")
|
||||
}
|
||||
if _, ok := ParseFacetField("nicht_existent"); ok {
|
||||
t.Fatalf("unbekannter feldname hätte abgelehnt werden müssen")
|
||||
}
|
||||
if _, ok := ParseFacetField("tenant_slug"); ok {
|
||||
t.Fatalf("ein reales, aber nicht-facettiertes suchfeld hätte abgelehnt werden müssen")
|
||||
}
|
||||
}
|
||||
|
||||
// TestNoDynamicFieldNameConstruction ist die geforderte Pflichtprüfung 2
|
||||
// (SRC-11): Code-Review bestätigt automatisiert, dass facets.go und
|
||||
// fields.go keinen dynamischen Spalten-/Tabellennamen per
|
||||
// String-Zusammenbau (fmt.Sprintf/+) erzeugen — Feldnamen kommen
|
||||
// ausschließlich aus den FacetField-Konstanten bzw. den statischen
|
||||
// FieldXxx-Konstanten dieses Pakets.
|
||||
func TestNoDynamicFieldNameConstruction(t *testing.T) {
|
||||
for _, file := range []string{"facets.go", "fields.go"} {
|
||||
src, err := os.ReadFile(file)
|
||||
if err != nil {
|
||||
t.Fatalf("%s lesen: %v", file, err)
|
||||
}
|
||||
for _, line := range strings.Split(string(src), "\n") {
|
||||
trimmed := strings.TrimSpace(line)
|
||||
if strings.HasPrefix(trimmed, "//") {
|
||||
continue // Kommentarzeilen dürfen den Begriff zur Erklärung nennen
|
||||
}
|
||||
if strings.Contains(line, "fmt.Sprintf") {
|
||||
t.Fatalf("%s darf kein fmt.Sprintf im Code verwenden (dynamische Feldnamenbildung verboten, SRC-11 Akzeptanzkriterium 1): %q", file, trimmed)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user