diff --git a/server/internal/app/route_sweep_test.go b/server/internal/app/route_sweep_test.go new file mode 100644 index 000000000..3c174e54d --- /dev/null +++ b/server/internal/app/route_sweep_test.go @@ -0,0 +1,130 @@ +package app + +import ( + "context" + "net/http" + "regexp" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/db/models" +) + +// sweepSkipped lists routes the route sweep must not drive: they end the +// session, delete the org the sweep runs in, or stream until the client goes +// away. +// +//nolint:gochecknoglobals // Effectively a constant table; Go has no const maps. +var sweepSkipped = map[string]string{ + "DELETE /api/v1/orgs/{org}": "deletes the org the sweep runs in", + "GET /api/v1/fake": "public self-test fixture that fails on purpose", + "POST /api/v1/auth/logout": "ends the sweep's own session", + "POST /api/v1/orgs/{org}/leave": "removes the sweep's own membership", +} + +var sweepParam = regexp.MustCompile(`\{[^}]+\}`) + +// sweepURL fills every path parameter of pattern: the org slug, the given +// uid for the *Uid / uid / check parameters, and a placeholder for the rest. +func sweepURL(pattern, orgSlug, uid string) string { + return sweepParam.ReplaceAllStringFunc(pattern, func(param string) string { + switch name := strings.Trim(param, "{}"); { + case name == "org": + return orgSlug + case name == "components": + return "x" + case name == "uid" || name == "check" || strings.HasSuffix(name, "Uid"): + return uid + default: + return "placeholder" + } + }) +} + +// TestEveryRouteAnswersWithoutAServerError walks the REAL route table and +// drives every route as the org owner (with a real check uid, then with a uid +// that exists nowhere, each with an empty and a malformed JSON body) and as an +// anonymous caller. Bad input and missing resources must be refused with a +// 4xx, never answered with a 5xx. +func TestEveryRouteAnswersWithoutAServerError(t *testing.T) { + t.Parallel() + r := require.New(t) + ctx := context.Background() + + env := newDemoEnv(t) + + check := models.NewCheck(env.org.UID, "sweep-check", "http") + r.NoError(env.server.dbService.CreateCheck(ctx, check)) + + type route struct{ method, pattern string } + + var routes []route + + r.NoError(env.server.router.Walk(func(method, pattern string) error { + if !strings.HasPrefix(pattern, "/api/v1/") || method == http.MethodOptions { + return nil + } + + if _, skip := sweepSkipped[method+" "+pattern]; !skip { + routes = append(routes, route{method, pattern}) + } + + return nil + })) + r.Greater(len(routes), 300, "the API route table looks implausibly small") + + const missingUID = "00000000-0000-4000-8000-000000000000" + + attempts := []struct { + token, uid, body string + }{ + {env.plainToken, check.UID, `{}`}, + {env.plainToken, missingUID, `{}`}, + {env.plainToken, missingUID, `{"name":`}, + {"", missingUID, `{}`}, + } + + var serverErrors []string + + for _, rt := range routes { + for _, at := range attempts { + status := env.sweep(rt.method, sweepURL(rt.pattern, env.org.Slug, at.uid), at.token, at.body) + // 503 is how an unconfigured integration (WebAuthn, bots) says so. + if status >= http.StatusInternalServerError && status != http.StatusServiceUnavailable { + serverErrors = append(serverErrors, rt.method+" "+rt.pattern) + } + } + } + + r.Empty(serverErrors, "routes answering 5xx to bad input or a missing resource") +} + +// sweep issues one request bounded by a short timeout (a streaming route that +// never ends counts as answered) and returns the status, or 0 on timeout. +func (e *demoEnv) sweep(method, path, token, body string) int { + e.t.Helper() + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, method, e.ts.URL+path, strings.NewReader(body)) + require.NoError(e.t, err) + + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + + req.Header.Set("Content-Type", "application/json") + + resp, err := e.ts.Client().Do(req) + if err != nil { + return 0 + } + + defer func() { _ = resp.Body.Close() }() + + return resp.StatusCode +} diff --git a/server/internal/db/dbcontract/closed.go b/server/internal/db/dbcontract/closed.go new file mode 100644 index 000000000..2b33acb9b --- /dev/null +++ b/server/internal/db/dbcontract/closed.go @@ -0,0 +1,30 @@ +package dbcontract + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/db" + "github.com/fclairamb/solidping/server/internal/testsupport" +) + +// RunClosed closes the service's connection pool, then calls every +// error-returning method with placeholder arguments. A database that has gone +// away must surface as an error from the method, never as a panic or a silent +// success, on every dialect. svc must be freshly initialized; it is unusable +// afterwards. +func RunClosed(t *testing.T, svc db.Service) { + t.Helper() + + must := require.New(t) + + must.NoError(svc.DB().Close()) + + called, failed := testsupport.CallErrorMethods(t, svc, "Close", "DB") + + // Most methods query the database before anything else, so the vast + // majority must report the closed pool. + must.Greater(called, 100) + must.Greater(failed, called/2, "methods that surfaced the closed database: %d of %d", failed, called) +} diff --git a/server/internal/db/postgres/contract_postgres_test.go b/server/internal/db/postgres/contract_postgres_test.go index 0ecea8f5e..50e1748f6 100644 --- a/server/internal/db/postgres/contract_postgres_test.go +++ b/server/internal/db/postgres/contract_postgres_test.go @@ -36,3 +36,32 @@ func TestDialectContract_Postgres(t *testing.T) { dbcontract.Run(t, s) } + +// portDialectContractClosed is the embedded-Postgres port of the closed-pool +// contract below; it must stay unique across packages. +const portDialectContractClosed = 15901 + +// TestDialectContractClosedDatabase_Postgres checks that every Service method +// reports a closed connection pool as an error on Postgres. +// +//nolint:paralleltest // shares dev-machine resources (embedded-postgres-go's pwfile extraction) with its siblings +func TestDialectContractClosedDatabase_Postgres(t *testing.T) { + if testing.Short() { + t.Skip("skipping embedded-postgres test in -short mode") + } + + ctx := t.Context() + + s, err := New(ctx, &Config{Embedded: true, Port: portDialectContractClosed, RunMode: runModeTest}) + if err != nil { + testsupport.PostgresUnavailable(t, err) + } + + t.Cleanup(func() { _ = s.Close() }) + + if initErr := s.Initialize(ctx); initErr != nil { + testsupport.PostgresInitFailed(t, initErr) + } + + dbcontract.RunClosed(t, s) +} diff --git a/server/internal/db/sqlite/contract_test.go b/server/internal/db/sqlite/contract_test.go index 4623d9c0c..a6a9733ad 100644 --- a/server/internal/db/sqlite/contract_test.go +++ b/server/internal/db/sqlite/contract_test.go @@ -21,3 +21,16 @@ func TestDialectContract(t *testing.T) { dbcontract.Run(t, svc) } + +// TestDialectContract_ClosedDatabase checks that every Service method reports +// a closed connection pool as an error. +func TestDialectContract_ClosedDatabase(t *testing.T) { + t.Parallel() + + ctx := t.Context() + svc, err := New(ctx, Config{InMemory: true}) + require.NoError(t, err) + require.NoError(t, svc.Initialize(ctx)) + + dbcontract.RunClosed(t, svc) +} diff --git a/server/internal/handlers/closeddb/closed_db_test.go b/server/internal/handlers/closeddb/closed_db_test.go new file mode 100644 index 000000000..eaf834453 --- /dev/null +++ b/server/internal/handlers/closeddb/closed_db_test.go @@ -0,0 +1,123 @@ +// Package closeddb_test checks, for every handler service, that a lost +// database connection comes back as an error from each method instead of a +// panic or a silent success. +package closeddb_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/config" + "github.com/fclairamb/solidping/server/internal/crypto/credentials" + "github.com/fclairamb/solidping/server/internal/db" + "github.com/fclairamb/solidping/server/internal/db/sqlite" + "github.com/fclairamb/solidping/server/internal/entitlements" + "github.com/fclairamb/solidping/server/internal/handlers/agents" + "github.com/fclairamb/solidping/server/internal/handlers/availability" + "github.com/fclairamb/solidping/server/internal/handlers/badges" + "github.com/fclairamb/solidping/server/internal/handlers/checkchannels" + "github.com/fclairamb/solidping/server/internal/handlers/checkdependencies" + "github.com/fclairamb/solidping/server/internal/handlers/checkgroups" + "github.com/fclairamb/solidping/server/internal/handlers/emailsuppressions" + "github.com/fclairamb/solidping/server/internal/handlers/escalationpolicies" + "github.com/fclairamb/solidping/server/internal/handlers/events" + "github.com/fclairamb/solidping/server/internal/handlers/files" + "github.com/fclairamb/solidping/server/internal/handlers/incidentnotifications" + "github.com/fclairamb/solidping/server/internal/handlers/incidentpublications" + "github.com/fclairamb/solidping/server/internal/handlers/integrations" + "github.com/fclairamb/solidping/server/internal/handlers/labels" + "github.com/fclairamb/solidping/server/internal/handlers/maintenancewindows" + "github.com/fclairamb/solidping/server/internal/handlers/members" + "github.com/fclairamb/solidping/server/internal/handlers/oncallschedules" + "github.com/fclairamb/solidping/server/internal/handlers/orgparams" + "github.com/fclairamb/solidping/server/internal/handlers/regions" + "github.com/fclairamb/solidping/server/internal/handlers/results" + "github.com/fclairamb/solidping/server/internal/handlers/severities" + "github.com/fclairamb/solidping/server/internal/handlers/slos" + "github.com/fclairamb/solidping/server/internal/handlers/statuspages" + "github.com/fclairamb/solidping/server/internal/handlers/statussubscribers" + "github.com/fclairamb/solidping/server/internal/handlers/statusupdates" + "github.com/fclairamb/solidping/server/internal/handlers/system" + "github.com/fclairamb/solidping/server/internal/handlers/uistate" + "github.com/fclairamb/solidping/server/internal/handlers/unsubscribe" + "github.com/fclairamb/solidping/server/internal/handlers/usernotifications" + "github.com/fclairamb/solidping/server/internal/testsupport" + "github.com/fclairamb/solidping/server/internal/utils/clock" +) + +func TestServicesReportAClosedDatabase(t *testing.T) { + t.Parallel() + + cfg := &config.Config{} + + newCreds := func(d db.Service) credentials.Service { + creds, err := credentials.NewService(nil, credentials.NewParamStore(d)) + require.NoError(t, err) + + return creds + } + + cases := []struct { + name string + make func(d db.Service) any + }{ + {"agents", func(d db.Service) any { return agents.NewService(d, newCreds(d), nil) }}, + {"availability", func(d db.Service) any { return availability.NewService(d, cfg) }}, + {"badges", func(d db.Service) any { return badges.NewService(d, cfg) }}, + {"checkchannels", func(d db.Service) any { return checkchannels.NewService(d) }}, + {"checkdependencies", func(d db.Service) any { return checkdependencies.NewService(d, 0) }}, + {"checkgroups", func(d db.Service) any { return checkgroups.NewService(d) }}, + {"emailsuppressions", func(d db.Service) any { return emailsuppressions.NewService(d) }}, + {"escalationpolicies", func(d db.Service) any { return escalationpolicies.NewService(d) }}, + {"events", func(d db.Service) any { return events.NewService(d) }}, + {"files", func(d db.Service) any { return files.NewService(d, cfg) }}, + {"incidentnotifications", func(d db.Service) any { return incidentnotifications.NewService(d) }}, + {"incidentpublications", func(d db.Service) any { + return incidentpublications.NewService(d, clock.Real{}, nil) + }}, + {"integrations", func(d db.Service) any { return integrations.NewService(d, newCreds(d), nil, cfg) }}, + {"labels", func(d db.Service) any { return labels.NewService(d) }}, + {"maintenancewindows", func(d db.Service) any { return maintenancewindows.NewService(d) }}, + {"members", func(d db.Service) any { return members.NewService(d) }}, + {"oncallschedules", func(d db.Service) any { return oncallschedules.NewService(d) }}, + {"orgparams", func(d db.Service) any { return orgparams.NewService(d) }}, + {"regions", func(d db.Service) any { return regions.NewService(d) }}, + {"results", func(d db.Service) any { return results.NewService(d, cfg) }}, + {"severities", func(d db.Service) any { return severities.NewService(d) }}, + {"slos", func(d db.Service) any { return slos.NewService(d, cfg, nil) }}, + {"statuspages", func(d db.Service) any { + return statuspages.NewService(d, cfg, entitlements.NewService(d, entitlements.DefaultsFor(""), 0)) + }}, + {"statussubscribers", func(d db.Service) any { return statussubscribers.NewService(d, newCreds(d)) }}, + {"statusupdates", func(d db.Service) any { return statusupdates.NewService(d) }}, + {"system", func(d db.Service) any { return system.NewService(d) }}, + {"uistate", func(d db.Service) any { return uistate.NewService(d) }}, + {"unsubscribe", func(d db.Service) any { return unsubscribe.NewService(d, []byte("secret")) }}, + {"usernotifications", func(d db.Service) any { return usernotifications.NewService(d, newCreds(d)) }}, + {"entitlements", func(d db.Service) any { + return entitlements.NewService(d, entitlements.DefaultsFor(""), 0) + }}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := t.Context() + + dbSvc, err := sqlite.New(ctx, sqlite.Config{InMemory: true}) + r.NoError(err) + r.NoError(dbSvc.Initialize(ctx)) + + svc := tc.make(dbSvc) + + r.NoError(dbSvc.DB().Close()) + + called, failed := testsupport.CallErrorMethods(t, svc) + r.Positive(called) + r.Positive(failed, "no method of %s reported the closed database (%d called)", tc.name, called) + }) + } +} diff --git a/server/internal/handlers/incidents/closed_db_test.go b/server/internal/handlers/incidents/closed_db_test.go new file mode 100644 index 000000000..86eda8ae0 --- /dev/null +++ b/server/internal/handlers/incidents/closed_db_test.go @@ -0,0 +1,37 @@ +package incidents_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/db/sqlite" + "github.com/fclairamb/solidping/server/internal/handlers/incidents" + "github.com/fclairamb/solidping/server/internal/jobs/jobsvc" + "github.com/fclairamb/solidping/server/internal/notifier" + "github.com/fclairamb/solidping/server/internal/testsupport" + "github.com/fclairamb/solidping/server/internal/utils/clock" +) + +// TestServiceReportsAClosedDatabase: every service method that touches the +// database answers a lost connection with an error instead of panicking or +// reporting success. +func TestServiceReportsAClosedDatabase(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := t.Context() + + dbSvc, err := sqlite.New(ctx, sqlite.Config{InMemory: true}) + r.NoError(err) + r.NoError(dbSvc.Initialize(ctx)) + + jobs := jobsvc.NewService(dbSvc.DB(), dbSvc, notifier.NewLocalEventNotifier(), nil) + svc := incidents.NewService(dbSvc, jobs, clock.Real{}, nil) + + r.NoError(dbSvc.DB().Close()) + + called, failed := testsupport.CallErrorMethods(t, svc) + r.Greater(called, 5) + r.Greater(failed, called/2, "methods that surfaced the closed database: %d of %d", failed, called) +} diff --git a/server/internal/testsupport/sweep.go b/server/internal/testsupport/sweep.go new file mode 100644 index 000000000..2b9a39154 --- /dev/null +++ b/server/internal/testsupport/sweep.go @@ -0,0 +1,106 @@ +package testsupport + +import ( + "context" + "reflect" + "testing" + "time" +) + +// sweepCallTimeout bounds one swept method call, so a method that blocks on +// its placeholder arguments cannot hang the suite. +const sweepCallTimeout = 10 * time.Second + +// CallErrorMethods calls every exported method of target whose last result is +// an error, with placeholder arguments (the test context, "x" strings, empty +// collections, zeroed structs behind non-nil pointers). It is meant for a +// target whose backing database has been closed: each method must then reach +// its first query and surface the failure as an error. A panic on the +// placeholders (a nil interface dereferenced before the first query) is +// recovered and counted as not failed. It returns how many methods were +// called and how many returned an error. +func CallErrorMethods(t *testing.T, target any, skip ...string) (int, int) { + t.Helper() + + skipped := make(map[string]bool, len(skip)) + for _, name := range skip { + skipped[name] = true + } + + value := reflect.ValueOf(target) + errType := reflect.TypeFor[error]() + + var called, failed int + + for i := range value.NumMethod() { + name := value.Type().Method(i).Name + method := value.Method(i) + sig := method.Type() + + if skipped[name] || sig.NumOut() == 0 || sig.Out(sig.NumOut()-1) != errType { + continue + } + + args := make([]reflect.Value, sig.NumIn()) + for j := range args { + args[j] = placeholder(t.Context(), sig.In(j)) + } + + called++ + + if callWithTimeout(method, args, sig.IsVariadic()) { + failed++ + } + } + + return called, failed +} + +// callWithTimeout reports whether the call returned a non-nil error. +func callWithTimeout(method reflect.Value, args []reflect.Value, variadic bool) bool { + done := make(chan bool, 1) + + go func() { + failed := false + + defer func() { + _ = recover() + done <- failed + }() + + var out []reflect.Value + if variadic { + out = method.CallSlice(args) + } else { + out = method.Call(args) + } + + failed = !out[len(out)-1].IsNil() + }() + + select { + case failed := <-done: + return failed + case <-time.After(sweepCallTimeout): + return false + } +} + +// placeholder builds a non-nil value of typ so a method gets past argument +// dereferences and reaches its first query. +func placeholder(ctx context.Context, typ reflect.Type) reflect.Value { + switch { + case typ == reflect.TypeFor[context.Context](): + return reflect.ValueOf(ctx) + case typ.Kind() == reflect.Pointer: + return reflect.New(typ.Elem()) + case typ.Kind() == reflect.String: + return reflect.ValueOf("x").Convert(typ) + case typ.Kind() == reflect.Slice: + return reflect.MakeSlice(typ, 0, 0) + case typ.Kind() == reflect.Map: + return reflect.MakeMap(typ) + default: + return reflect.Zero(typ) + } +}