Document review fixes and harden mail actions
This commit is contained in:
parent
4f8a441d51
commit
21aa6e39e1
5 changed files with 101 additions and 3 deletions
57
REVIEW-FIXES.md
Normal file
57
REVIEW-FIXES.md
Normal file
|
|
@ -0,0 +1,57 @@
|
||||||
|
# Mail-Graveyard Review-Fixes
|
||||||
|
|
||||||
|
Stand: 2026-07-14
|
||||||
|
|
||||||
|
## Richtung
|
||||||
|
|
||||||
|
Der Migrationskern soll auf `emersion/go-imap/v2` umgestellt werden. Der aktuell handgeschriebene `simpleIMAP`-Parser ist zwar schlank, aber fuer ein Beweis-Archiv zu riskant: Ordnernamen, Flags, INTERNALDATE und FETCH-Varianten muessen korrekt und serverunabhaengig verarbeitet werden.
|
||||||
|
|
||||||
|
## P0 - Migrationsintegritaet
|
||||||
|
|
||||||
|
1. `simpleIMAP` durch `emersion/go-imap/v2` ersetzen.
|
||||||
|
- Betrifft `backend/04-imap-source.go` und `backend/05-imap-target.go`.
|
||||||
|
- Ziel: IMAP UTF-7 korrekt dekodieren, `FETCH` robust parsen, Flags korrekt normalisieren, APPEND RFC-konform senden.
|
||||||
|
|
||||||
|
2. IMAP UTF-7 Ordnernamen korrekt behandeln.
|
||||||
|
- Aktuelles Risiko: Rollen-Erkennung fuer Papierkorb/Geloescht/Gesendet scheitert bei Alt-Providern.
|
||||||
|
- Folge: Zielordner werden doppelt angelegt.
|
||||||
|
|
||||||
|
3. `\Recent` niemals per APPEND setzen.
|
||||||
|
- `\Recent` ist serververwaltet und darf nicht vom Client appended werden.
|
||||||
|
|
||||||
|
4. INTERNALDATE robust parsen.
|
||||||
|
- Ein- und zweistellige Tage muessen funktionieren.
|
||||||
|
- Bei Parse-Fehlern nicht still auf `time.Now()` fallen, sondern melden oder kontrolliert degradieren.
|
||||||
|
|
||||||
|
5. FETCH-Reihenfolge unabhaengig parsen.
|
||||||
|
- FLAGS, INTERNALDATE und BODY koennen serverseitig in anderer Reihenfolge kommen.
|
||||||
|
|
||||||
|
6. Ordnerweise Streaming statt kompletter Ordner im RAM.
|
||||||
|
- Ziel: grosse Postfaecher nicht in den Speicher laden.
|
||||||
|
|
||||||
|
7. POP3-Fallback verdrahten.
|
||||||
|
- `src_proto=pop3` darf nicht weiter in IMAP laufen.
|
||||||
|
|
||||||
|
## P1 - Sicherheit und Rollen
|
||||||
|
|
||||||
|
1. SMTP-Header-Injection verhindern.
|
||||||
|
- Erledigt: Betreff, From-Header und To-Header werden auf CR/LF geprueft.
|
||||||
|
|
||||||
|
2. Schreibende Viewer-Aktionen rollenbegrenzen.
|
||||||
|
- Erledigt: `/mail/send` und `/view/manual-copy` sind vorerst nur fuer Verwalter/Admins erreichbar.
|
||||||
|
- Offene Produktentscheidung: Wenn normale Benutzer weiterhin eingeschraenkt weiterleiten duerfen, braucht es einen separaten, begrenzten Forward-Endpunkt statt freiem Compose-SMTP.
|
||||||
|
|
||||||
|
## P2 - Lose Enden
|
||||||
|
|
||||||
|
1. `/view/forward` und `ForwardMessage()` entfernen oder sauber in den neuen Forward-Flow integrieren.
|
||||||
|
2. Sync-Dropdowns entweder implementieren oder sichtbar als "noch nicht aktiv" kennzeichnen.
|
||||||
|
3. Stale `TODO Codex`-Kommentare an fertigen Handlern bereinigen.
|
||||||
|
4. Tote Render-/Helper-Funktionen entfernen, sobald die Transfer-Workbench stabil ist.
|
||||||
|
|
||||||
|
## Verifikation pro Fix-Serie
|
||||||
|
|
||||||
|
- `go test ./...`
|
||||||
|
- Live-Test gegen einen kleinen IMAP-Account mit Umlautordnern.
|
||||||
|
- Live-Test gegen ein Alt-Provider-Postfach ohne SPECIAL-USE.
|
||||||
|
- Grossordner-Test mit Speicherbeobachtung.
|
||||||
|
- Rollen-Test: User darf nur Archiv lesen; Verwalter/Admin duerfen senden, kopieren und verwalten.
|
||||||
|
|
@ -65,6 +65,9 @@ func RegisterRoutes(mux *http.ServeMux) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func mailSendHandler(w http.ResponseWriter, r *http.Request) {
|
func mailSendHandler(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if !requireManager(w, r) {
|
||||||
|
return
|
||||||
|
}
|
||||||
if r.Method != http.MethodPost {
|
if r.Method != http.MethodPost {
|
||||||
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
|
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
|
||||||
return
|
return
|
||||||
|
|
|
||||||
|
|
@ -271,6 +271,9 @@ func archiveFolderSortKey(name string) string {
|
||||||
}
|
}
|
||||||
|
|
||||||
func manualCopyHandler(w http.ResponseWriter, r *http.Request) {
|
func manualCopyHandler(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if !requireManager(w, r) {
|
||||||
|
return
|
||||||
|
}
|
||||||
if r.Method != http.MethodPost {
|
if r.Method != http.MethodPost {
|
||||||
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
|
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
|
||||||
return
|
return
|
||||||
|
|
|
||||||
|
|
@ -17,13 +17,25 @@ func SendPlainMail(to []string, subject, body string) error {
|
||||||
if strings.TrimSpace(cfg.Host) == "" || cfg.Port == 0 || strings.TrimSpace(cfg.From) == "" {
|
if strings.TrimSpace(cfg.Host) == "" || cfg.Port == 0 || strings.TrimSpace(cfg.From) == "" {
|
||||||
return fmt.Errorf("forward_smtp ist nicht vollstaendig konfiguriert")
|
return fmt.Errorf("forward_smtp ist nicht vollstaendig konfiguriert")
|
||||||
}
|
}
|
||||||
|
fromHeader, err := cleanMailHeader(cfg.From)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("ungueltiger Absender: %w", err)
|
||||||
|
}
|
||||||
|
subjectHeader, err := cleanMailHeader(subject)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("ungueltiger Betreff: %w", err)
|
||||||
|
}
|
||||||
|
toHeader, err := cleanMailHeader(strings.Join(to, ", "))
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("ungueltiger Empfaenger: %w", err)
|
||||||
|
}
|
||||||
from := cfg.From
|
from := cfg.From
|
||||||
if strings.TrimSpace(cfg.User) != "" {
|
if strings.TrimSpace(cfg.User) != "" {
|
||||||
from = cfg.User
|
from = cfg.User
|
||||||
}
|
}
|
||||||
message := []byte("From: " + cfg.From + "\r\n" +
|
message := []byte("From: " + fromHeader + "\r\n" +
|
||||||
"To: " + strings.Join(to, ", ") + "\r\n" +
|
"To: " + toHeader + "\r\n" +
|
||||||
"Subject: " + subject + "\r\n" +
|
"Subject: " + subjectHeader + "\r\n" +
|
||||||
"Content-Type: text/plain; charset=utf-8\r\n" +
|
"Content-Type: text/plain; charset=utf-8\r\n" +
|
||||||
"\r\n" + body)
|
"\r\n" + body)
|
||||||
addr := net.JoinHostPort(cfg.Host, fmt.Sprint(cfg.Port))
|
addr := net.JoinHostPort(cfg.Host, fmt.Sprint(cfg.Port))
|
||||||
|
|
@ -37,6 +49,17 @@ func SendPlainMail(to []string, subject, body string) error {
|
||||||
return sendStartTLS(addr, cfg.Host, auth, from, to, message)
|
return sendStartTLS(addr, cfg.Host, auth, from, to, message)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func cleanMailHeader(value string) (string, error) {
|
||||||
|
cleaned := strings.TrimSpace(value)
|
||||||
|
if cleaned == "" {
|
||||||
|
return "", nil
|
||||||
|
}
|
||||||
|
if strings.ContainsAny(cleaned, "\r\n") {
|
||||||
|
return "", fmt.Errorf("Header darf keine Zeilenumbrueche enthalten")
|
||||||
|
}
|
||||||
|
return cleaned, nil
|
||||||
|
}
|
||||||
|
|
||||||
func sendStartTLS(addr, host string, auth smtp.Auth, from string, to []string, message []byte) error {
|
func sendStartTLS(addr, host string, auth smtp.Auth, from string, to []string, message []byte) error {
|
||||||
c, err := smtp.Dial(addr)
|
c, err := smtp.Dial(addr)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
||||||
12
backend/09-smtp_test.go
Normal file
12
backend/09-smtp_test.go
Normal file
|
|
@ -0,0 +1,12 @@
|
||||||
|
package backend
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
func TestCleanMailHeaderRejectsLineBreaks(t *testing.T) {
|
||||||
|
if _, err := cleanMailHeader("Betreff\r\nBcc: attacker@example.com"); err == nil {
|
||||||
|
t.Fatal("expected CRLF header injection to be rejected")
|
||||||
|
}
|
||||||
|
if got, err := cleanMailHeader(" Normaler Betreff "); err != nil || got != "Normaler Betreff" {
|
||||||
|
t.Fatalf("cleanMailHeader returned %q, %v", got, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Add a link
Reference in a new issue