From 3057719fa1ab306a54c341d655acae8becd3bbbb Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Sun, 4 Oct 2026 01:43:17 +0200 Subject: [PATCH] test: every widget validation limit, bucket and request edge holds one step either side - TestFeedbackEdges: the submit rules and the me/hidden boolean at each limit and one past it, the 60/10 per-minute buckets, the 413 over the body cap, a screenshot without an uploads bucket, missing services or tables answering 500, the G15Office job registration, the settings import as the CLI runs it, and the migrations rolling back and forward --- feedback_edges_test.go | 359 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 359 insertions(+) create mode 100644 feedback_edges_test.go diff --git a/feedback_edges_test.go b/feedback_edges_test.go new file mode 100644 index 0000000..c17034d --- /dev/null +++ b/feedback_edges_test.go @@ -0,0 +1,359 @@ +package feedback + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "git.golem15.com/golem15/sm-feedback-plugin/classes" + "git.golem15.com/golem15/sm-feedback-plugin/console" + "git.golem15.com/golem15/sm-feedback-plugin/controllers/api" + "git.golem15.com/golem15/sm-feedback-plugin/internal/pgtest" + "git.golem15.com/golem15/sm-feedback-plugin/updates" + userclasses "git.golem15.com/golem15/sm-user-plugin/classes" + usermodels "git.golem15.com/golem15/sm-user-plugin/models" + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/bonfire" + "git.golem15.com/golem15/summercms/modules/conga" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/lagoon/attach" + "git.golem15.com/golem15/summercms/modules/party" + "git.golem15.com/golem15/summercms/modules/surf" + "gorm.io/gorm" +) + +// smallHandler is newHandler with the router's body cap at limit bytes +// and, when bucket is false, no uploads bucket published. +func smallHandler(t *testing.T, limit int, bucket bool) (http.Handler, *backpack.App, *gorm.DB) { + t.Helper() + gdb := pgtest.DB(t) + cfg := testConfig(t) + for k, v := range map[string]any{ + "storage.uploads.bucket_url": "file://" + t.TempDir(), + "http.body_limits.default_bytes": limit, + "http.body_limits.upload_bytes": limit, + } { + if err := cfg.Set(k, v); err != nil { + t.Fatal(err) + } + } + app := backpack.New(cfg) + if err := lagoon.Publish(app, pgtest.SQL(t), gdb); err != nil { + t.Fatal(err) + } + if bucket { + b, err := attach.OpenBucket(t.Context(), cfg) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = b.Close() }) + if err := attach.Publish(app, b); err != nil { + t.Fatal(err) + } + } + plugins, err := party.Activate(app, pgtest.PluginIDs) + if err != nil { + t.Fatal(err) + } + h, err := surf.Assemble(app, plugins) + if err != nil { + t.Fatal(err) + } + return h, app, gdb +} + +// TestFeedbackEdges pins every validation limit of the widget routes one +// step either side (the submit rules and the me/hidden boolean), the two +// per-IP buckets (60 config reads and 10 submissions a minute), and the +// edges of the request handling: the 413 over the body cap, a screenshot +// without an uploads bucket, a missing database or settings table, an +// unauthenticated or failing preference write, the job registration and +// the user payload listener. Each failure is a 4xx or 500 answer, never a +// stored row. +func TestFeedbackEdges(t *testing.T) { + ctx := context.Background() + gdb := pgtest.DB(t) + + t.Run("submit-rules-one-step-either-side", func(t *testing.T) { + email255 := strings.Repeat("a", 64) + "@" + strings.Repeat("b", 63) + "." + strings.Repeat("c", 63) + "." + strings.Repeat("d", 61) + base := map[string]any{"message": "m", "type": "bug", "page_url": "https://x.test/"} + for _, c := range []struct { + field string + ok any + bad any + }{ + {"message", strings.Repeat("ż", 5000), strings.Repeat("ż", 5001)}, + {"page_url", strings.Repeat("p", 2000), strings.Repeat("p", 2001)}, + {"user_agent", strings.Repeat("u", 500), strings.Repeat("u", 501)}, + {"console_log", strings.Repeat("c", 20000), strings.Repeat("c", 20001)}, + {"email", email255, "a" + email255}, + {"type", "feature", "question"}, + } { + for _, v := range []struct { + value any + want bool + }{{c.ok, true}, {c.bad, false}} { + in := map[string]any{} + for k, x := range base { + in[k] = x + } + in[c.field] = v.value + errs, err := lagoon.ValidateRequest(ctx, gdb, in, api.SubmitRules, nil) + if err != nil { + t.Fatal(err) + } + if passed := len(errs) == 0; passed != v.want { + t.Errorf("%s of %d chars: passed %v, want %v (%v)", c.field, len([]rune(v.value.(string))), passed, v.want, errs) + } + } + } + for _, missing := range []string{"message", "type", "page_url"} { + in := map[string]any{"message": "m", "type": "bug", "page_url": "u"} + delete(in, missing) + if errs, _ := lagoon.ValidateRequest(ctx, gdb, in, api.SubmitRules, nil); errs[missing] == nil { + t.Errorf("a missing %s passed", missing) + } + } + }) + + t.Run("me-hidden-boolean", func(t *testing.T) { + for _, c := range []struct { + v any + want bool + }{ + {true, true}, {false, true}, {json.Number("1"), true}, {json.Number("0"), true}, {"1", true}, {"0", true}, + {json.Number("2"), false}, {"yes", false}, {"true", false}, {"", false}, {nil, false}, + } { + errs, err := lagoon.ValidateRequest(ctx, gdb, map[string]any{"hidden": c.v}, api.MeHiddenRules, nil) + if err != nil { + t.Fatal(err) + } + if passed := len(errs) == 0; passed != c.want { + t.Errorf("hidden=%#v passed %v, want %v", c.v, passed, c.want) + } + } + }) + + t.Run("buckets-60-and-10", func(t *testing.T) { + b := (&Plugin{}).Buckets() + if b[BucketConfig].Max != 60 || b[BucketConfig].Decay != time.Minute || b[BucketSubmit].Max != 10 || b[BucketSubmit].Decay != time.Minute { + t.Fatalf("buckets %+v", b) + } + h, _, gdb := newHandler(t) + resetSettings(t, gdb, "", nil) + for i := 1; i <= 61; i++ { + rec := do(t, h, httptest.NewRequest(http.MethodGet, "/_feedback/api/v1/"+testWidgetKey+"/config", nil)) + if i <= 60 && rec.Code != http.StatusOK { + t.Fatalf("config read %d = %d", i, rec.Code) + } + if i == 61 && rec.Code != http.StatusTooManyRequests { + t.Fatalf("config read 61 = %d, want 429", rec.Code) + } + } + }) + + submitURL := "/_feedback/api/v1/" + testWidgetKey + "/submit" + t.Run("over-the-body-cap", func(t *testing.T) { + h, app, gdb := smallHandler(t, 2048, true) + resetSettings(t, gdb, "", nil) + before := countRows(t, gdb, `SELECT COUNT(*) FROM golem15_feedback_submissions`) + rec := do(t, h, multipartRequest(t, submitURL, validParts(map[string]string{"console_log": strings.Repeat("c", 8192)}), "")) + if rec.Code != http.StatusRequestEntityTooLarge || rec.Body.String() != `{"error":"Payload too large"}` { + t.Fatalf("oversized submit = %d %s", rec.Code, rec.Body) + } + u := createUser(t, gdb, "edges-hidden@example.test") + big := `{"hidden":true,"pad":"` + strings.Repeat("x", 4096) + `"}` + if rec := putHidden(t, h, bearer(t, app, u), big); rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("oversized me/hidden = %d %s", rec.Code, rec.Body) + } + if after := countRows(t, gdb, `SELECT COUNT(*) FROM golem15_feedback_submissions`); after != before { + t.Fatalf("an oversized submit stored %d rows", after-before) + } + }) + + t.Run("a-screenshot-without-an-uploads-bucket", func(t *testing.T) { + h, _, gdb := smallHandler(t, 32<<20, false) + resetSettings(t, gdb, "", nil) + parts := append(validParts(nil), part{name: "screenshot", filename: "s.png", ctype: "image/png", data: pngBytes(t, 0)}) + if rec := do(t, h, multipartRequest(t, submitURL, parts, "")); rec.Code != http.StatusInternalServerError { + t.Fatalf("no bucket = %d %s", rec.Code, rec.Body) + } + }) + + t.Run("form-and-json-bodies", func(t *testing.T) { + h, app, gdb := newHandler(t) + u := createUser(t, gdb, "edges-forms@example.test") + auth := bearer(t, app, u) + for _, c := range []struct { + ctype, body string + code int + want string + }{ + {"application/x-www-form-urlencoded", "hidden=1", 200, `{"hidden":true}`}, + {"application/json", `{"hidden":0}`, 200, `{"hidden":false}`}, + // PHP decodes 1.0 as a float, which the strict boolean rule refuses. + {"application/json", `{"hidden":1.0}`, 422, ""}, + {"application/json", `{not json`, 422, ""}, + {"application/json", ``, 422, ""}, + } { + req := httptest.NewRequest(http.MethodPut, "/_feedback/api/v1/me/hidden", strings.NewReader(c.body)) + req.Header.Set("Content-Type", c.ctype) + req.Header.Set("Authorization", auth) + rec := do(t, h, req) + if rec.Code != c.code || (c.want != "" && rec.Body.String() != c.want) { + t.Errorf("%s %q = %d %s", c.ctype, c.body, rec.Code, rec.Body) + } + } + }) + + t.Run("handlers-without-their-services", func(t *testing.T) { + bare := backpack.New(testConfig(t)) + for name, hf := range map[string]http.HandlerFunc{"config": api.Config(bare), "submit": api.Submit(bare), "me-hidden": api.MeHidden(bare)} { + rec := httptest.NewRecorder() + hf(rec, httptest.NewRequest(http.MethodPost, "/x", nil)) + if rec.Code != http.StatusInternalServerError { + t.Errorf("%s without a database = %d %s", name, rec.Code, rec.Body) + } + } + h, app, gdb := newHandler(t) + resetSettings(t, gdb, "", nil) + u := createUser(t, gdb, "edges-tables@example.test") + auth := bearer(t, app, u) + rename := func(table string, fn func()) { + t.Helper() + if err := gdb.Exec(`ALTER TABLE ` + table + ` RENAME TO ` + table + `_away`).Error; err != nil { + t.Fatal(err) + } + defer gdb.Exec(`ALTER TABLE ` + table + `_away RENAME TO ` + table) + fn() + } + rename("golem15_feedback_settings", func() { + if rec := do(t, h, httptest.NewRequest(http.MethodGet, "/_feedback/api/v1/"+testWidgetKey+"/config", nil)); rec.Code != http.StatusInternalServerError { + t.Errorf("config without settings = %d %s", rec.Code, rec.Body) + } + }) + rename("golem15_feedback_user_preferences", func() { + if rec := putHidden(t, h, auth, `{"hidden":true}`); rec.Code != http.StatusInternalServerError { + t.Errorf("me/hidden without the preferences table = %d %s", rec.Code, rec.Body) + } + // The user payload listener reports the failure too. + if _, err := app.Events.Collect(ctx, &userclasses.GetApiArrayEvent{User: u}); err == nil { + t.Error("the payload listener swallowed a database error") + } + }) + if got, err := app.Events.Collect(ctx, &userclasses.GetApiArrayEvent{}); err != nil || got["feedback_widget_hidden"] != nil { + t.Errorf("a payload without a user = %v %v", got, err) + } + withoutDB := backpack.New(testConfig(t)) + if err := (&Plugin{}).Boot(withoutDB); err != nil { + t.Fatal(err) + } + if _, err := withoutDB.Events.Collect(ctx, &userclasses.GetApiArrayEvent{User: &usermodels.User{ID: 1}}); err == nil { + t.Error("the payload listener ran without a database") + } + }) + + t.Run("g15office-job-registration-and-resolution", func(t *testing.T) { + jobs := (&Plugin{}).Jobs() + info, ok := conga.Describe(jobs[0]) + if len(jobs) != 1 || !ok || info.Queue != classes.SyncG15OfficeQueue || info.MaxAttempts != 3 { + t.Fatalf("jobs %+v %v", info, ok) + } + if err := (&Plugin{}).syncG15Office(ctx, classes.SyncG15OfficeArgs{SubmissionID: 1}); err == nil { + t.Error("the job ran without an app") + } + if err := (&Plugin{app: backpack.New(testConfig(t))}).syncG15Office(ctx, classes.SyncG15OfficeArgs{SubmissionID: 1}); err == nil { + t.Error("the job ran without a database") + } + _, app, _ := newHandler(t) + // A submission that is gone is nothing to file. + if err := (&Plugin{app: app}).syncG15Office(ctx, classes.SyncG15OfficeArgs{SubmissionID: 2147483000}); err != nil { + t.Errorf("a missing submission = %v", err) + } + }) + + t.Run("import-command-as-the-cli-runs-it", func(t *testing.T) { + run := func(app *backpack.App, force bool) (string, error) { + var buf bytes.Buffer + err := console.ImportSettings(app).Run(ctx, forceInput(force), bonfire.NewOutput(nil, &buf, &buf)) + return buf.String(), err + } + if _, err := run(nil, false); err == nil { + t.Fatal("a nil app must fail") + } + if _, err := run(backpack.New(testConfig(t)), false); err == nil { + t.Fatal("an app without a database must fail") + } + _, app, gdb := newHandler(t) + if err := gdb.Exec(`DELETE FROM golem15_feedback_settings`).Error; err != nil { + t.Fatal(err) + } + if err := gdb.Exec(`DROP TABLE IF EXISTS system_settings`).Error; err != nil { + t.Fatal(err) + } + t.Cleanup(func() { gdb.Exec(`DROP TABLE IF EXISTS system_settings`) }) + if out, err := run(app, false); err != nil || !strings.Contains(out, "nothing to import") { + t.Fatalf("no source = %q %v", out, err) + } + if err := gdb.Exec(`CREATE TABLE system_settings (id SERIAL PRIMARY KEY, item TEXT, value TEXT)`).Error; err != nil { + t.Fatal(err) + } + // Switches and labels as PHP stores them: booleans, numbers, null. + value := `{"enabled":true,"allow_hide":0,"widget_key":"wk_EdgesImport000000000000000000000","position":7,"color_primary":null,"label_title_en":false}` + if err := gdb.Exec(`INSERT INTO system_settings (item, value) VALUES ('golem15_feedback_settings', ?)`, value).Error; err != nil { + t.Fatal(err) + } + if out, err := run(app, false); err != nil || strings.Contains(out, "nothing to import") { + t.Fatalf("import = %q %v", out, err) + } + if out, err := run(app, false); err == nil || !strings.Contains(out, "--force") && !strings.Contains(strings.ToLower(out), "custom") { + t.Fatalf("a second import = %q %v", out, err) + } + if out, err := run(app, true); err != nil { + t.Fatalf("a forced import = %q %v", out, err) + } + if err := gdb.Exec(`UPDATE system_settings SET value = '{not json'`).Error; err != nil { + t.Fatal(err) + } + if _, err := run(app, true); err == nil { + t.Error("malformed settings were imported") + } + }) + + t.Run("migrations-roll-back-and-forward", func(t *testing.T) { + tx := gdb.Begin() + defer tx.Rollback() + ms := updates.All() + for i := len(ms) - 1; i >= 0; i-- { + if err := ms[i].Rollback(tx); err != nil { + t.Fatalf("rollback %s: %v", ms[i].ID, err) + } + } + if tx.Migrator().HasTable("golem15_feedback_submissions") { + t.Fatal("the rollback left the submissions table") + } + for _, m := range ms { + if err := m.Migrate(tx); err != nil { + t.Fatalf("migrate %s: %v", m.ID, err) + } + } + }) +} + +// forceInput is a bonfire.Input with the --force switch. +type forceInput bool + +func (f forceInput) Args() []string { return nil } +func (f forceInput) Argument(string) (string, bool) { return "", false } +func (f forceInput) Flags(string) []string { return nil } +func (f forceInput) Flag(name string) (string, bool) { + if name == "force" && bool(f) { + return "true", true + } + return "", false +}