diff --git a/docs/API-23-PRUEFPROTOKOLL.md b/docs/API-23-PRUEFPROTOKOLL.md new file mode 100644 index 00000000..2c193103 --- /dev/null +++ b/docs/API-23-PRUEFPROTOKOLL.md @@ -0,0 +1,51 @@ +# API-23 – Prüfprotokoll: HTTP-Client mit Zeitlimit im Core-Root-Modul + +Nachtrag zu API-19 (QA-15-Befund F-11): das Core-Root-Modul selbst war bei der +damaligen Umstellung nicht erfasst. + +## Umsetzung + +- `internal/httpclient` (neu, Core-Root-Modul): identisches Muster wie + `dms/internal/httpclient`, `archive/internal/httpclient`, + `mail/internal/httpclient` — `New(timeout)` liefert einen `*http.Client` mit + Zeitlimit und `CheckRedirect`, das `http.ErrUseLastResponse` zurückgibt + (keine Weiterleitungen); `Default()` liefert den gemeinsamen Client mit + `DefaultTimeout = 15s`. +- `internal/statuspage/statuspage.go`: `NewHTTPChecker` nutzt jetzt + `httpclient.Default()` statt eines ungebundenen `&http.Client{}`. +- `internal/moduletrust/fetch.go`: `FetchJWKSFromURL` nutzt bei `client == nil` + jetzt `httpclient.Default()` statt `http.DefaultClient`. +- Architekturtest `TestNoUnboundedDefaultClientInProductionCode` in + `internal/httpclient/httpclient_test.go`: durchsucht nur `internal/` und + `cmd/` (die eigenständigen Module `dms/`, `archive/`, `mail/`, `connect/`, + `ai/`, `workflow/` liegen zwar im selben Repo, sind aber eigene Go-Module mit + eigenem Architekturtest und daher hier bewusst ausgeklammert). Erkennt + zusätzlich zu `http.DefaultClient`/`http.Get|Post|Head|PostForm(` auch den + ungebundenen `http.Client{}` (genau das Muster, das in `statuspage.go` + gefunden wurde). + +## Prüfungen + +| # | Prüfung | Ergebnis | +|---|---|---| +| 1 | Hängender Testserver: Aufruf über `internal/statuspage`/`internal/moduletrust` bricht nach ~15s ab statt unbegrenzt zu warten | **bestanden** – `TestClientGivesUpOnHangingServer` (via `internal/httpclient`, das beide Aufrufer jetzt nutzen): `httptest`-Server hängt endlos, `New(200ms).Get(...)` liefert nach Zeitlimit einen Fehler statt unbegrenzt zu warten | +| 2 | Testserver mit Weiterleitung: Aufruf folgt der Weiterleitung nicht | **bestanden** – `TestClientDoesNotFollowRedirects`: Weiterleitungsziel-Handler ruft `t.Error` auf, falls er je erreicht wird; `Default().Do(req)` liefert den 307 der Weiterleitungsantwort selbst, das Ziel wird nie aufgerufen | +| 3 | Architekturtest schlägt fehl, wenn `http.DefaultClient`/`http.Client{}` wieder eingeführt wird (echt provoziert, nicht nur behauptet) | **bestanden** – real provoziert: `internal/moduletrust/fetch.go` testweise auf `client = http.DefaultClient` zurückgesetzt, `go test ./internal/httpclient/... -run TestNoUnbounded` schlägt mit exakter Datei:Zeile-Angabe fehl; Datei danach wiederhergestellt, `go build ./...` wieder sauber | + +## Build/Test-Ergebnis (192.168.1.204) + +``` +go build ./... -> clean +go vet ./... -> clean +go test ./internal/httpclient/... ./internal/statuspage/... \ + ./internal/moduletrust/... -v -> alle PASS + (Integrationstests ohne + TEST_ADMIN_DSN übersprungen, + unverändert durch dieses Ticket) +``` + +## Gesamtergebnis + +**Bestanden.** Alle drei Akzeptanzkriterien und alle drei Pflichtprüfungen +real erfüllt. Damit ist F-11/API-19 nun auch für das Core-Root-Modul +nachgezogen; `dms`, `archive`, `mail` bereits durch API-19 abgedeckt. diff --git a/internal/httpclient/httpclient.go b/internal/httpclient/httpclient.go new file mode 100644 index 00000000..23200113 --- /dev/null +++ b/internal/httpclient/httpclient.go @@ -0,0 +1,29 @@ +// Package httpclient liefert den HTTP-Client fuer ausgehende Dienstaufrufe +// im Core-Root-Modul (Nachtrag zu QA-15 F-11/API-19): Zeitlimit statt +// endlosem Warten auf einen haengenden Nachbardienst, und keine +// Weiterleitungen (ein Service-Token wuerde sonst an das +// Weiterleitungsziel mitgesendet). +package httpclient + +import ( + "net/http" + "time" +) + +const DefaultTimeout = 15 * time.Second + +var shared = New(DefaultTimeout) + +// Default ist der gemeinsame Client mit DefaultTimeout. +func Default() *http.Client { return shared } + +// New liefert einen Client mit dem angegebenen Zeitlimit, der Weiterleitungen +// nicht folgt (die 3xx-Antwort wird zurueckgegeben). +func New(timeout time.Duration) *http.Client { + return &http.Client{ + Timeout: timeout, + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + } +} diff --git a/internal/httpclient/httpclient_test.go b/internal/httpclient/httpclient_test.go new file mode 100644 index 00000000..e38ae411 --- /dev/null +++ b/internal/httpclient/httpclient_test.go @@ -0,0 +1,110 @@ +package httpclient + +import ( + "io/fs" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + "time" +) + +func TestDefaultHasTimeout(t *testing.T) { + if Default().Timeout != DefaultTimeout || DefaultTimeout <= 0 { + t.Fatalf("timeout = %s", Default().Timeout) + } +} + +func TestClientGivesUpOnHangingServer(t *testing.T) { + release := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { <-release })) + defer srv.Close() + defer close(release) + + start := time.Now() + resp, err := New(200 * time.Millisecond).Get(srv.URL) + if err == nil { + _ = resp.Body.Close() + t.Fatal("kein fehler bei haengendem server") + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("aufruf dauerte %s", elapsed) + } +} + +func TestClientDoesNotFollowRedirects(t *testing.T) { + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("weiterleitungsziel wurde aufgerufen (Service-Token-Leck)") + })) + defer target.Close() + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL, http.StatusTemporaryRedirect) + })) + defer redirector.Close() + + req, _ := http.NewRequest(http.MethodPost, redirector.URL, strings.NewReader("x")) + req.Header.Set("X-Service-Token", "geheim") + resp, err := Default().Do(req) + if err != nil { + t.Fatalf("do: %v", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusTemporaryRedirect { + t.Fatalf("status = %d, erwartet 307", resp.StatusCode) + } +} + +// Architekturtest: im Produktivcode dieses Moduls gibt es keine Aufrufe ueber +// den Standard-Client ohne Zeitlimit, auch keinen ungebundenen http.Client{}. +func TestNoUnboundedDefaultClientInProductionCode(t *testing.T) { + repoRoot, err := filepath.Abs(filepath.Join("..", "..")) + if err != nil { + t.Fatal(err) + } + // Core-root-Modul lebt am Repo-Wurzelverzeichnis neben den eigenstaendigen + // Modulen (dms/, archive/, mail/, ...) - nur internal/ und cmd/ gehoeren + // zu diesem Modul, der Rest wird von den jeweiligen Modul-Architekturtests + // abgedeckt. + scanDirs := []string{"internal", "cmd"} + forbidden := regexp.MustCompile(`http\.DefaultClient|http\.(Get|Post|Head|PostForm)\(|http\.Client\{\}`) + for _, dir := range scanDirs { + root := filepath.Join(repoRoot, dir) + if _, err := os.Stat(root); os.IsNotExist(err) { + continue + } + err = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if n := d.Name(); n == "node_modules" || n == "testdata" || n == ".git" { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + for i, line := range strings.Split(string(data), "\n") { + if t := strings.TrimSpace(line); strings.HasPrefix(t, "//") { + continue + } + if forbidden.MatchString(line) { + rel, _ := filepath.Rel(repoRoot, path) + t.Errorf("%s:%d nutzt den Standard-Client ohne Zeitlimit: %s", rel, i+1, strings.TrimSpace(line)) + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + } +} diff --git a/internal/moduletrust/fetch.go b/internal/moduletrust/fetch.go index 458b2b0c..850f0b5c 100644 --- a/internal/moduletrust/fetch.go +++ b/internal/moduletrust/fetch.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "net/http" + + "gitea.perlbach24.de/scripte/nexarch/internal/httpclient" ) // FetchJWKSFromURL baut eine KeyFetchFunc, die einen JWKS-Endpunkt (siehe @@ -16,7 +18,7 @@ import ( // bestimmt, wie oft tatsaechlich abgerufen wird (nicht bei jeder Verify()). func FetchJWKSFromURL(client *http.Client, url string) KeyFetchFunc { if client == nil { - client = http.DefaultClient + client = httpclient.Default() } return func(ctx context.Context) (map[string]ed25519.PublicKey, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) diff --git a/internal/statuspage/statuspage.go b/internal/statuspage/statuspage.go index 73753ec9..6dbb002a 100644 --- a/internal/statuspage/statuspage.go +++ b/internal/statuspage/statuspage.go @@ -18,6 +18,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "gitea.perlbach24.de/scripte/nexarch/internal/apierror" + "gitea.perlbach24.de/scripte/nexarch/internal/httpclient" ) // Status ist der aggregierte Zustand EINES Moduls zu einem Zeitpunkt. @@ -181,7 +182,7 @@ type HTTPChecker struct { } func NewHTTPChecker(timeout time.Duration) *HTTPChecker { - return &HTTPChecker{Client: &http.Client{}, Timeout: timeout} + return &HTTPChecker{Client: httpclient.Default(), Timeout: timeout} } func (c *HTTPChecker) Check(ctx context.Context, url string) Status {