API-21: Architekturtest auch fuer das Archive-Modul

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

archive/internal/archtest/errorleak.go

@@ -0,0 +1,221 @@
+// Package archtest scannt den Quellbaum eines Moduls statisch (AST, keine
+// Typprüfung) und erzwingt zwei QA-15/API-20/API-21-Regeln, die sich zur
+// Laufzeit allein nicht garantieren lassen: kein Fehlertext direkt in eine
+// HTTP-Antwort, und jeder HTTP-Dienst läuft über httpharden.
+package archtest
+
+import (
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// AllowDirective markiert eine bewusste, begründete Ausnahme. Die Zeile mit
+// dem Fund (oder die Zeile direkt darüber) muss den Kommentar
+// "// archtest:allow <Begründung>" tragen, sonst zählt der Fund.
+const AllowDirective = "archtest:allow"
+
+// Finding ist ein einzelner Regelverstoß mit Fundstelle.
+type Finding struct {
+	File string
+	Line int
+	Rule string
+	Text string
+}
+
+func (f Finding) String() string {
+	return fmt.Sprintf("%s:%d: [%s] %s", f.File, f.Line, f.Rule, f.Text)
+}
+
+// Scan durchläuft alle .go-Dateien unter root (Modulwurzel), ausser Tests,
+// vendor und den explizit ausgenommenen Paketen (relative Pfade ab root,
+// z.B. "internal/apierror", "internal/httpharden" - dort ist das Schreiben
+// von Fehlertexten bzw. der rohe ListenAndServe-Aufruf genau die erlaubte
+// Implementierung der Regel selbst, kein Verstoss dagegen).
+func Scan(root string, exemptPackages []string) ([]Finding, error) {
+	var findings []Finding
+	fset := token.NewFileSet()
+
+	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() {
+			base := info.Name()
+			if base == "vendor" || base == "node_modules" || strings.HasPrefix(base, ".") {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+		if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
+			return nil
+		}
+		rel, relErr := filepath.Rel(root, path)
+		if relErr != nil {
+			return relErr
+		}
+		relDir := filepath.ToSlash(filepath.Dir(rel))
+		for _, ex := range exemptPackages {
+			if relDir == ex || strings.HasPrefix(relDir, ex+"/") {
+				return nil
+			}
+		}
+
+		src, readErr := os.ReadFile(path)
+		if readErr != nil {
+			return readErr
+		}
+		file, parseErr := parser.ParseFile(fset, path, src, parser.ParseComments)
+		if parseErr != nil {
+			return fmt.Errorf("parsen von %s: %w", path, parseErr)
+		}
+
+		allowedLines := allowedLinesInFile(fset, file)
+		lines := strings.Split(string(src), "\n")
+
+		ast.Inspect(file, func(n ast.Node) bool {
+			call, ok := n.(*ast.CallExpr)
+			if !ok {
+				return true
+			}
+			pos := fset.Position(call.Pos())
+			if allowedLines[pos.Line] || (pos.Line > 0 && allowedLines[pos.Line-1]) {
+				return true
+			}
+
+			if isErrorLeakSink(call) {
+				findings = append(findings, Finding{
+					File: rel, Line: pos.Line, Rule: "err-in-response",
+					Text: strings.TrimSpace(lineText(lines, pos.Line)),
+				})
+			}
+			if isRawListenAndServe(call) {
+				findings = append(findings, Finding{
+					File: rel, Line: pos.Line, Rule: "raw-listen-and-serve",
+					Text: strings.TrimSpace(lineText(lines, pos.Line)),
+				})
+			}
+			return true
+		})
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return findings, nil
+}
+
+func lineText(lines []string, n int) string {
+	if n-1 < 0 || n-1 >= len(lines) {
+		return ""
+	}
+	return lines[n-1]
+}
+
+// allowedLinesInFile sammelt alle Zeilen, deren Kommentar AllowDirective
+// enthaelt (die Kommentarzeile selbst zaehlt als "allowed", ausserdem prueft
+// Scan zusaetzlich die Vorzeile - deckt sowohl "// archtest:allow ..." direkt
+// ueber dem Aufruf als auch einen Trailing-Kommentar in derselben Zeile ab).
+func allowedLinesInFile(fset *token.FileSet, file *ast.File) map[int]bool {
+	allowed := map[int]bool{}
+	for _, cg := range file.Comments {
+		for _, c := range cg.List {
+			if strings.Contains(c.Text, AllowDirective) {
+				allowed[fset.Position(c.Pos()).Line] = true
+			}
+		}
+	}
+	return allowed
+}
+
+// isErrorLeakSink erkennt Aufrufe der Form w.Write(...), fmt.Fprintf(w, ...),
+// fmt.Fprint(w, ...), fmt.Fprintln(w, ...) und json.NewEncoder(w).Encode(...),
+// bei denen ein Argument (an beliebiger Verschachtelungstiefe) syntaktisch
+// ein X.Error()-Aufruf ist. Ohne Typpruefung wird "w" als Namenskonvention
+// fuer http.ResponseWriter genutzt (durchgaengig so in diesem Code
+// verwendet, u.a. in apierror.Write selbst) - das erzeugt bewusst false
+// negatives statt false positives bei anders benannten Variablen, die dann
+// ueber Codereview auffallen muessten, nicht ueber diesen Test.
+func isErrorLeakSink(call *ast.CallExpr) bool {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return false
+	}
+
+	switch sel.Sel.Name {
+	case "Write":
+		if ident, ok := sel.X.(*ast.Ident); !ok || ident.Name != "w" {
+			return false
+		}
+	case "Fprintf", "Fprint", "Fprintln":
+		pkg, ok := sel.X.(*ast.Ident)
+		if !ok || pkg.Name != "fmt" || len(call.Args) == 0 {
+			return false
+		}
+		if ident, ok := call.Args[0].(*ast.Ident); !ok || ident.Name != "w" {
+			return false
+		}
+	case "Encode":
+		inner, ok := sel.X.(*ast.CallExpr)
+		if !ok {
+			return false
+		}
+		innerSel, ok := inner.Fun.(*ast.SelectorExpr)
+		if !ok || innerSel.Sel.Name != "NewEncoder" || len(inner.Args) == 0 {
+			return false
+		}
+		if ident, ok := inner.Args[0].(*ast.Ident); !ok || ident.Name != "w" {
+			return false
+		}
+	default:
+		return false
+	}
+
+	for _, arg := range call.Args {
+		if containsErrorCall(arg) {
+			return true
+		}
+	}
+	return false
+}
+
+// containsErrorCall sucht rekursiv nach einem Aufruf ....Error() in arg.
+func containsErrorCall(n ast.Node) bool {
+	found := false
+	ast.Inspect(n, func(node ast.Node) bool {
+		if found {
+			return false
+		}
+		call, ok := node.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		if sel, ok := call.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Error" && len(call.Args) == 0 {
+			found = true
+			return false
+		}
+		return true
+	})
+	return found
+}
+
+// isRawListenAndServe erkennt http.ListenAndServe(...) und
+// (&http.Server{...}).ListenAndServe() bzw. srv.ListenAndServe() auf einer
+// lokal deklarierten *http.Server-Variablen ausserhalb von httpharden.
+// Aus Praezisionsgruenden (kein Typchecker) wird nur der eindeutige Fall
+// http.ListenAndServe(...) erkannt, das reicht fuer diesen Codebestand
+// (alle bekannten Dienste rufen entweder httpharden.ListenAndServe*, oder
+// direkt http.ListenAndServe auf - letzteres ist genau der zu verbietende
+// Fall).
+func isRawListenAndServe(call *ast.CallExpr) bool {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok || sel.Sel.Name != "ListenAndServe" {
+		return false
+	}
+	pkg, ok := sel.X.(*ast.Ident)
+	return ok && pkg.Name == "http"
+}

archive/internal/archtest/errorleak_test.go

@@ -0,0 +1,80 @@
+package archtest
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+)
+
+// moduleRoot findet die Modulwurzel (die Datei-Ebene mit go.mod), ausgehend
+// vom Pfad dieser Testdatei - unabhaengig davon, mit welchem Arbeitsverzeichnis
+// "go test" aufgerufen wird.
+func moduleRoot(t *testing.T) string {
+	t.Helper()
+	_, thisFile, _, ok := runtime.Caller(0)
+	if !ok {
+		t.Fatal("eigenen Dateipfad nicht ermitteln koennen")
+	}
+	dir := filepath.Dir(thisFile)
+	for {
+		if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
+			return dir
+		}
+		parent := filepath.Dir(dir)
+		if parent == dir {
+			t.Fatal("go.mod nicht gefunden")
+		}
+		dir = parent
+	}
+}
+
+// exempt sind Pakete, in denen das Schreiben eines Fehlertexts in die
+// Antwort bzw. der rohe ListenAndServe-Aufruf genau die erlaubte
+// Implementierung der Regel selbst ist (API-20/API-21), kein Verstoss.
+// Das Archive-Modul hat keine weiteren, physisch verschachtelten Go-Module.
+var exempt = []string{"internal/apierror", "internal/httpharden"}
+
+// TestNoErrorTextLeaksIntoHTTPResponses ist Akzeptanzkriterium 1: kein
+// err.Error() direkt in einer HTTP-Antwort, ausser ueber apierror.
+func TestNoErrorTextLeaksIntoHTTPResponses(t *testing.T) {
+	root := moduleRoot(t)
+	findings, err := Scan(root, exempt)
+	if err != nil {
+		t.Fatalf("Quellbaum scannen: %v", err)
+	}
+	var leaks []Finding
+	for _, f := range findings {
+		if f.Rule == "err-in-response" {
+			leaks = append(leaks, f)
+		}
+	}
+	if len(leaks) > 0 {
+		t.Errorf("%d Fehlertext-Leck(s) in HTTP-Antworten gefunden (nicht ueber apierror, keine archtest:allow-Ausnahme):", len(leaks))
+		for _, f := range leaks {
+			t.Errorf("  %s", f)
+		}
+	}
+}
+
+// TestAllHTTPServersUseHTTPHarden ist Akzeptanzkriterium 2: kein Dienst
+// startet direkt ueber http.ListenAndServe an httpharden vorbei.
+func TestAllHTTPServersUseHTTPHarden(t *testing.T) {
+	root := moduleRoot(t)
+	findings, err := Scan(root, exempt)
+	if err != nil {
+		t.Fatalf("Quellbaum scannen: %v", err)
+	}
+	var raw []Finding
+	for _, f := range findings {
+		if f.Rule == "raw-listen-and-serve" {
+			raw = append(raw, f)
+		}
+	}
+	if len(raw) > 0 {
+		t.Errorf("%d Aufruf(e) von http.ListenAndServe ausserhalb von httpharden gefunden (nicht apierror.Normalize-gehaertet, keine archtest:allow-Ausnahme):", len(raw))
+		for _, f := range raw {
+			t.Errorf("  %s", f)
+		}
+	}
+}