From c076b4c059f9c35c791f888bc473a438166f01c5 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 5 Oct 2026 14:30:31 +0200 Subject: [PATCH] test(12.1-05): threat test for the Phase 12.1 framework contracts and the first gate stages - TestPhase121Threats: one subtest per mitigated threat T-12.1-01 to T-12.1-15 - roster fixture: sentinel names and knobs for failing hooks and providers - scripts/check-phase12.1.sh: fail-closed go test detector, --self-test and --security --- modules/cabana/phase121_fixture_test.go | 122 ++++++- modules/cabana/phase121_threats_test.go | 447 ++++++++++++++++++++++++ scripts/check-phase12.1.sh | 269 ++++++++++++++ 3 files changed, 827 insertions(+), 11 deletions(-) create mode 100644 modules/cabana/phase121_threats_test.go create mode 100755 scripts/check-phase12.1.sh diff --git a/modules/cabana/phase121_fixture_test.go b/modules/cabana/phase121_fixture_test.go index 76e4140..468fb40 100644 --- a/modules/cabana/phase121_fixture_test.go +++ b/modules/cabana/phase121_fixture_test.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "sync" + "sync/atomic" "testing" "testing/fstest" "time" @@ -190,11 +191,50 @@ func (s *rosterSpy) takeBulk() []pact.AdminBulkActionInput { return out } +// rosterKnobs switch on failures of the controller's providers, which get no +// record to carry a sentinel name. A nil pointer switches nothing on. +type rosterKnobs struct { + // permissionOptions makes AdminPermissionOptions fail. + permissionOptions atomic.Bool + // permissionValues makes AdminPermissionValues fail. + permissionValues atomic.Bool + // relationLocks makes AdminRelationLocks fail. + relationLocks atomic.Bool + // slowArchive, when set, runs inside the archive bulk action after the + // rows were locked (concurrency tests). + slowArchive atomic.Pointer[func()] +} + +// The sentinel names below make one hook of the roster controller misbehave +// for the person who carries the name. +const ( + // rosterKeep: FormBeforeDelete refuses with a ForbiddenError. + rosterKeep = "Keep" + // rosterKeepAfter: FormAfterUpdate and FormAfterDelete refuse after the + // row was written or removed. + rosterKeepAfter = "KeepAfter" + // rosterShort: ListRowStates answers one entry too few. + rosterShort = "Short" + // rosterStateErr: ListRowStates fails with a plain error. + rosterStateErr = "StateErr" + // rosterAppliesErr: the activate record action's Applies fails. + rosterAppliesErr = "AppliesErr" + // rosterCrash: the archive bulk action fails with a plain error. + rosterCrash = "Crash" + // rosterRunErr: the reinstate record action fails with a plain error + // after its write. + rosterRunErr = "RunErr" + // rosterDenyCreate and rosterDenyAfterCreate: the create hooks refuse. + rosterDenyCreate = "DenyCreate" + rosterDenyAfterCreate = "DenyAfterCreate" +) + // rosterPlugin is the acme.roster fixture plugin. fsys, when set, replaces // the fixture tree (boot-error tests). type rosterPlugin struct { - spy *rosterSpy - fsys fs.FS + spy *rosterSpy + knobs *rosterKnobs + fsys fs.FS // db is the handle the controller reads filter choices and locked tags // with outside a transaction. db *gorm.DB @@ -208,7 +248,7 @@ func (rosterPlugin) Requires() []string { return nil } func (rosterPlugin) Register(*backpack.App) error { return nil } func (rosterPlugin) Boot(*backpack.App) error { return nil } func (p rosterPlugin) AdminControllers() []pact.AdminController { - return []pact.AdminController{rosterController{spy: p.spy, db: p.db, relations: p.relations}} + return []pact.AdminController{rosterController{spy: p.spy, knobs: p.knobs, db: p.db, relations: p.relations}} } func (rosterPlugin) Permissions() []pact.Permission { return []pact.Permission{{Code: "acme.roster.access", Roles: []string{"developer"}}, {Code: "acme.roster.manage", Roles: []string{"developer"}}} @@ -235,6 +275,7 @@ func (rosterPlugin) LangFS() fs.FS { type rosterController struct { spy *rosterSpy + knobs *rosterKnobs db *gorm.DB relations func([]cabana.FieldRelationContract) []cabana.FieldRelationContract } @@ -275,6 +316,9 @@ func (c rosterController) handle(ctx context.Context) *gorm.DB { // AdminRelationLocks locks the staff tag for an administrator without // acme.roster.manage. func (c rosterController) AdminRelationLocks(ctx context.Context, field string) (cabana.RelationLock, error) { + if c.knobs != nil && c.knobs.relationLocks.Load() { + return cabana.RelationLock{}, fmt.Errorf("the lock table said hunter2") + } principal, _ := bouncer.User(ctx) if field != "tags" || cabana.Allows(principal, []string{"acme.roster.manage"}) { return cabana.RelationLock{}, nil @@ -339,8 +383,13 @@ func (c rosterController) ListRowStates(ctx context.Context, db *gorm.DB, record if person.DeletedAt.Valid { out[i] = append(out[i], pact.RowStateDeleted) } - if person.Name == "Odd" { + switch person.Name { + case "Odd": out[i] = append(out[i], pact.RowState("starred")) + case rosterShort: + return out[:len(out)-1], nil + case rosterStateErr: + return nil, fmt.Errorf("the state table said hunter2") } } return out, nil @@ -387,7 +436,10 @@ var rosterPermissionCodes = []cabana.PermissionOption{ // AdminPermissionOptions serves the permission editor's options per // administrator. -func (rosterController) AdminPermissionOptions(ctx context.Context, field string) ([]cabana.PermissionOption, error) { +func (c rosterController) AdminPermissionOptions(ctx context.Context, field string) ([]cabana.PermissionOption, error) { + if c.knobs != nil && c.knobs.permissionOptions.Load() { + return nil, fmt.Errorf("the permission table said hunter2") + } if field != "permissions" { return nil, fmt.Errorf("unknown permission field %s", field) } @@ -402,7 +454,10 @@ func (rosterController) AdminPermissionOptions(ctx context.Context, field string } // AdminPermissionValues reads the stored JSON object. -func (rosterController) AdminPermissionValues(_ context.Context, _ string, record any) (map[string]int, error) { +func (c rosterController) AdminPermissionValues(_ context.Context, _ string, record any) (map[string]int, error) { + if c.knobs != nil && c.knobs.permissionValues.Load() { + return nil, fmt.Errorf("the permission column said hunter2") + } person := record.(*rosterPerson) out := map[string]int{} if person.Permissions == nil || *person.Permissions == "" { @@ -466,11 +521,33 @@ func (c rosterController) FormBeforeCreate(ctx context.Context, model any) error model.(*rosterPerson).Tenant = "acme" storePassword(model.(*rosterPerson), values) delete(values, "notify") + if model.(*rosterPerson).Name == rosterDenyCreate { + return rosterRefused + } return nil } -func (c rosterController) FormAfterCreate(ctx context.Context, _ any) error { +func (c rosterController) FormAfterCreate(ctx context.Context, model any) error { c.spy.recordVirtual("after-create", ctx) + if model.(*rosterPerson).Name == rosterDenyAfterCreate { + return rosterRefused + } + return nil +} + +// FormAfterUpdate refuses the name KeepAfter after the row was written. +func (rosterController) FormAfterUpdate(_ context.Context, model any) error { + if model.(*rosterPerson).Name == rosterKeepAfter { + return rosterRefused + } + return nil +} + +// FormBeforeDelete refuses the person named Keep. +func (rosterController) FormBeforeDelete(_ context.Context, model any) error { + if model.(*rosterPerson).Name == rosterKeep { + return rosterRefused + } return nil } @@ -501,7 +578,14 @@ func (rosterController) FormAfterDelete(ctx context.Context, model any) error { if !ok { return fmt.Errorf("no transaction on the context") } - return tx.Unscoped().Delete(model).Error + if err := tx.Unscoped().Delete(model).Error; err != nil { + return err + } + // Refused after the row was removed: the transaction must bring it back. + if model.(*rosterPerson).Name == rosterKeepAfter { + return rosterRefused + } + return nil } // AdminBulkActions: activate needs acme.roster.manage and sets active on the @@ -540,12 +624,20 @@ func (c rosterController) AdminBulkActions() []pact.AdminBulkAction { if !ok { return pact.AdminBulkActionResult{}, fmt.Errorf("no transaction on the context") } + if c.knobs != nil { + if wait := c.knobs.slowArchive.Load(); wait != nil { + (*wait)() + } + } for _, record := range in.Records { // A refusal after earlier rows were written: the whole // selection must roll back. if record.(*rosterPerson).Name == rosterLocked { return pact.AdminBulkActionResult{}, rosterRefused } + if record.(*rosterPerson).Name == rosterCrash { + return pact.AdminBulkActionResult{}, fmt.Errorf("the archive said hunter2") + } if err := tx.Delete(record).Error; err != nil { return pact.AdminBulkActionResult{}, err } @@ -563,6 +655,9 @@ func (c rosterController) AdminRecordActions() []pact.AdminRecordAction { Name: "activate", Label: "acme.roster::lang.people.activate", Permissions: []string{"acme.roster.manage"}, Applies: func(_ context.Context, record any) (bool, error) { + if record.(*rosterPerson).Name == rosterAppliesErr { + return false, fmt.Errorf("the applies check said hunter2") + } return !record.(*rosterPerson).Active, nil }, Run: func(ctx context.Context, in pact.AdminRecordActionInput) (pact.AdminRecordActionResult, error) { @@ -594,6 +689,9 @@ func (c rosterController) AdminRecordActions() []pact.AdminRecordAction { if in.Record.(*rosterPerson).Name == rosterLocked { return pact.AdminRecordActionResult{}, rosterRefused } + if in.Record.(*rosterPerson).Name == rosterRunErr { + return pact.AdminRecordActionResult{}, fmt.Errorf("the reinstate said hunter2") + } return pact.AdminRecordActionResult{}, nil }, }} @@ -604,7 +702,8 @@ func (c rosterController) AdminRecordActions() []pact.AdminRecordAction { // token), limited (acme.roster.access only), cookie and cookie-only. type rosterEnv struct { *actEnv - spy *rosterSpy + spy *rosterSpy + knobs *rosterKnobs } func newRosterEnv(t *testing.T) (*rosterEnv, *gorm.DB) { @@ -656,7 +755,8 @@ func newRosterEnvWith(t *testing.T, configure func(*rosterPlugin)) (*rosterEnv, t.Fatal(err) } spy := &rosterSpy{} - plugin := rosterPlugin{spy: spy, db: gdb} + knobs := &rosterKnobs{} + plugin := rosterPlugin{spy: spy, knobs: knobs, db: gdb} if configure != nil { configure(&plugin) } @@ -668,7 +768,7 @@ func newRosterEnvWith(t *testing.T, configure func(*rosterPlugin)) (*rosterEnv, if err != nil { t.Fatal(err) } - env := &rosterEnv{actEnv: &actEnv{h: h}, spy: spy} + env := &rosterEnv{actEnv: &actEnv{h: h}, spy: spy, knobs: knobs} rec := postJSON(t, h, adminAPI("/auth/login"), map[string]string{"login": login, "password": adminTestPassword}) if rec.Code != http.StatusOK { t.Fatalf("login status=%d body=%s", rec.Code, rec.Body.String()) diff --git a/modules/cabana/phase121_threats_test.go b/modules/cabana/phase121_threats_test.go new file mode 100644 index 0000000..ac4b68a --- /dev/null +++ b/modules/cabana/phase121_threats_test.go @@ -0,0 +1,447 @@ +package cabana_test + +import ( + "context" + "encoding/json" + "fmt" + "log/slog" + "net/http" + "reflect" + "strings" + "sync" + "testing" + + "git.golem15.com/golem15/summercms/modules/cabana" +) + +// rosterIDs is the body of a bulk request. +func rosterIDs(ids ...uint) string { + raw, _ := json.Marshal(map[string]any{"ids": ids}) + return string(raw) +} + +// rosterPath is the admin path of one person, with an optional suffix. +func rosterPath(id uint, suffix string) string { + return fmt.Sprintf("%s/%d%s", rosterPeople, id, suffix) +} + +// rosterPair is the password pair every roster create needs. +const rosterPair = `"password":"long-enough-1","password_confirmation":"long-enough-1"` + +// logCapture keeps the records written to the default logger while it is +// installed. +type logCapture struct { + mu sync.Mutex + records []string +} + +func (c *logCapture) Enabled(context.Context, slog.Level) bool { return true } +func (c *logCapture) WithAttrs([]slog.Attr) slog.Handler { return c } +func (c *logCapture) WithGroup(string) slog.Handler { return c } +func (c *logCapture) Handle(_ context.Context, record slog.Record) error { + var line strings.Builder + line.WriteString(record.Message) + record.Attrs(func(attr slog.Attr) bool { + fmt.Fprintf(&line, " %s=%v", attr.Key, attr.Value.Any()) + return true + }) + c.mu.Lock() + c.records = append(c.records, line.String()) + c.mu.Unlock() + return nil +} + +// matching returns the captured lines that start with prefix. +func (c *logCapture) matching(prefix string) []string { + c.mu.Lock() + defer c.mu.Unlock() + var out []string + for _, line := range c.records { + if strings.HasPrefix(line, prefix) { + out = append(out, line) + } + } + return out +} + +// captureLog installs a capturing default logger for the rest of the test. +func captureLog(t *testing.T) *logCapture { + t.Helper() + capture := &logCapture{} + previous := slog.Default() + slog.SetDefault(slog.New(capture)) + t.Cleanup(func() { slog.SetDefault(previous) }) + return capture +} + +// TestPhase121Threats has one subtest per mitigated framework threat of +// Phase 12.1, named by its id. Each asserts the protection through the admin +// API on the acme.roster fixture, so removing the protection fails the +// subtest on an assertion (scripts/check-phase12.1.sh --removal). +// +// T-12.1-16 (a preview URL in plugin YAML that points at a foreign route) is +// mitigated in the SPA and pinned by the vitest case "backstop: mapWinterUrl +// maps preview/:id to the preview route" in admin/tests/app/winterUrl.test.ts. +// T-12.1-17 (the tag) is a release step and has no code path. +func TestPhase121Threats(t *testing.T) { + t.Run("T-12.1-01", func(t *testing.T) { + env, gdb := newRosterEnv(t) + own := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Own"}) + foreign := rosterInsert(t, gdb, rosterPerson{Tenant: "other", Name: "Foreign"}) + // Only out-of-scope ids: nothing reaches Run and nothing changes. + rec := env.expect(t, http.StatusOK, http.MethodPost, rosterPeople+"/bulk/activate", rosterIDs(foreign), "bearer") + if result := rosterBulkResult(t, rec); result.Affected != 0 { + t.Fatalf("out-of-scope selection affected %d rows", result.Affected) + } + // A mixed selection is refused as a whole. + rec = env.expect(t, http.StatusConflict, http.MethodPost, rosterPeople+"/bulk/activate", rosterIDs(own, foreign), "bearer") + actErrorCode(t, rec.Body.Bytes(), "conflict") + if calls := env.spy.takeBulk(); len(calls) != 0 { + t.Fatalf("Run was called with %d selections for ids outside the list scope", len(calls)) + } + if rosterLoad(t, gdb, own).Active || rosterLoad(t, gdb, foreign).Active { + t.Fatal("a refused selection changed a row") + } + }) + + t.Run("T-12.1-02", func(t *testing.T) { + env, gdb := newRosterEnv(t) + idle := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Idle"}) + // Undeclared names, and names registered in the other namespace only. + for _, name := range []string{"missing", "reinstate", "delete", "create"} { + env.expect(t, http.StatusNotFound, http.MethodPost, rosterPeople+"/bulk/"+name, rosterIDs(idle), "bearer") + } + for _, name := range []string{"missing", "archive", "delete", "create"} { + env.expect(t, http.StatusNotFound, http.MethodPost, rosterPath(idle, "/actions/"+name), `{}`, "bearer") + } + // Declared, but the administrator lacks the action's own permission. + rec := env.expect(t, http.StatusForbidden, http.MethodPost, rosterPeople+"/bulk/activate", rosterIDs(idle), "limited") + actErrorCode(t, rec.Body.Bytes(), "forbidden") + rec = env.expect(t, http.StatusForbidden, http.MethodPost, rosterPath(idle, "/actions/activate"), `{}`, "limited") + actErrorCode(t, rec.Body.Bytes(), "forbidden") + if rosterLoad(t, gdb, idle).Active { + t.Fatal("an action ran without its permission") + } + if calls, one := env.spy.takeBulk(), env.spy.takeRecord(); len(calls) != 0 || len(one) != 0 { + t.Fatalf("a refused action reached the plugin: bulk=%d record=%d", len(calls), len(one)) + } + // The same action is absent from what that administrator is offered. + if got := rosterBulkNames(t, env, "limited"); !reflect.DeepEqual(got, []string{"delete", "archive"}) { + t.Fatalf("limited admin is offered bulk actions %v", got) + } + if got := rosterOffered(t, env, idle, "limited"); len(got) != 0 { + t.Fatalf("limited admin is offered record actions %v", got) + } + if got := rosterOffered(t, env, idle, "bearer"); !reflect.DeepEqual(got, []string{"activate"}) { + t.Fatalf("full admin is offered record actions %v", got) + } + }) + + t.Run("T-12.1-03", func(t *testing.T) { + env, gdb := newRosterEnv(t) + idle := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Idle"}) + for _, rel := range []string{rosterPeople + "/bulk/activate", rosterPath(idle, "/actions/activate")} { + body := `{}` + if strings.Contains(rel, "/bulk/") { + body = rosterIDs(idle) + } + rec := env.expect(t, http.StatusForbidden, http.MethodPost, rel, body, "cookie-only") + actErrorCode(t, rec.Body.Bytes(), "forbidden") + if rosterLoad(t, gdb, idle).Active { + t.Fatalf("%s ran for a cookie request without X-Requested-With", rel) + } + } + // The same cookie with the header is accepted. + env.expect(t, http.StatusOK, http.MethodPost, rosterPath(idle, "/actions/activate"), `{}`, "cookie") + if !rosterLoad(t, gdb, idle).Active { + t.Fatal("the cookie request with the header did not run") + } + }) + + t.Run("T-12.1-04", func(t *testing.T) { + env, gdb := newRosterEnv(t) + active := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Active", Active: true}) + foreign := rosterInsert(t, gdb, rosterPerson{Tenant: "other", Name: "Foreign"}) + rec := env.expect(t, http.StatusNotFound, http.MethodPost, rosterPath(foreign, "/actions/activate"), `{}`, "bearer") + actErrorCode(t, rec.Body.Bytes(), "not_found") + rec = env.expect(t, http.StatusConflict, http.MethodPost, rosterPath(active, "/actions/activate"), `{}`, "bearer") + actErrorCode(t, rec.Body.Bytes(), "conflict") + if calls := env.spy.takeRecord(); len(calls) != 0 { + t.Fatalf("Run was called %d times for a record outside the scope or state", len(calls)) + } + if rosterLoad(t, gdb, foreign).Active { + t.Fatal("an out-of-scope record was changed") + } + }) + + t.Run("T-12.1-05", func(t *testing.T) { + env, gdb := newRosterEnv(t) + first := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "First"}) + second := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Second"}) + crash := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: rosterCrash}) + // The action soft-deletes the first two rows and fails on the last. + env.expect(t, http.StatusInternalServerError, http.MethodPost, rosterPeople+"/bulk/archive", rosterIDs(first, second, crash), "bearer") + for _, id := range []uint{first, second, crash} { + if rosterLoad(t, gdb, id).DeletedAt.Valid { + t.Fatalf("row %d kept the write of a bulk action that failed on the last row", id) + } + } + if calls := env.spy.takeBulk(); len(calls) != 1 || len(calls[0].Records) != 3 { + t.Fatalf("the action did not run over the three rows: %+v", calls) + } + }) + + t.Run("T-12.1-06", func(t *testing.T) { + env, gdb := newRosterEnv(t) + id := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Ada"}) + rec := env.expect(t, http.StatusInternalServerError, http.MethodPut, rosterPath(id, ""), `{"name":"Boom"}`, "bearer") + if got := rosterError(t, rec); got.Code != "error" || len(got.Details) != 0 { + t.Fatalf("500 error = %+v", got) + } + for _, leak := range []string{"hunter2", "roster database"} { + if strings.Contains(rec.Body.String(), leak) { + t.Fatalf("the 500 body carries the hook's error text %q: %s", leak, rec.Body.String()) + } + } + crash := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: rosterCrash}) + rec = env.expect(t, http.StatusInternalServerError, http.MethodPost, rosterPeople+"/bulk/archive", rosterIDs(crash), "bearer") + if strings.Contains(rec.Body.String(), "hunter2") { + t.Fatalf("the 500 body of a bulk action carries its error text: %s", rec.Body.String()) + } + // A ForbiddenError is the one error whose text is the plugin's to show. + rec = env.expect(t, http.StatusForbidden, http.MethodPut, rosterPath(id, ""), `{"name":"Reserved"}`, "bearer") + if got := rosterError(t, rec); got.Code != "forbidden" || got.Message != "You may not rename this person." { + t.Fatalf("403 error = %+v", got) + } + }) + + t.Run("T-12.1-07", func(t *testing.T) { + env, gdb := newRosterEnv(t) + odd := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Odd", Active: true}) + idle := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Idle"}) + body, raw := rosterList(t, env, "") + if strings.Contains(raw, "starred") { + t.Fatalf("a row state outside the fixed set was sent: %s", raw) + } + if _, ok := body.Meta.RowStates[fmt.Sprint(odd)]; ok { + t.Fatalf("a row with only an unknown state is listed: %v", body.Meta.RowStates) + } + if got := body.Meta.RowStates[fmt.Sprint(idle)]; !reflect.DeepEqual(got, []string{"disabled"}) { + t.Fatalf("known state = %v", got) + } + }) + + t.Run("T-12.1-08", func(t *testing.T) { + env, gdb := newRosterEnv(t) + idle := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Secretname", Email: "secret@example.test"}) + logs := captureLog(t) + env.expect(t, http.StatusOK, http.MethodPost, rosterPeople+"/bulk/activate", rosterIDs(idle), "bearer") + banned := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Secretname", Email: "secret@example.test", Active: true, Banned: true}) + env.expect(t, http.StatusOK, http.MethodPost, rosterPath(banned, "/actions/reinstate"), `{}`, "bearer") + + bulk := logs.matching("cabana: admin bulk action") + if len(bulk) != 1 || !strings.Contains(bulk[0], "controller=acme.roster.people") || !strings.Contains(bulk[0], "action=activate") || !strings.Contains(bulk[0], "affected=1") { + t.Fatalf("bulk action log lines = %q", bulk) + } + record := logs.matching("cabana: admin record action") + if len(record) != 1 || !strings.Contains(record[0], "action=reinstate") || !strings.Contains(record[0], fmt.Sprintf("record_id=%d", banned)) { + t.Fatalf("record action log lines = %q", record) + } + for _, line := range append(bulk, record...) { + if !strings.Contains(line, "admin_id=") || strings.Contains(line, "admin_id=0") { + t.Fatalf("log line lacks the admin id: %q", line) + } + if strings.Contains(line, "Secretname") || strings.Contains(line, "secret@example.test") { + t.Fatalf("log line carries record contents: %q", line) + } + } + // A refused request writes no action line. + env.expect(t, http.StatusForbidden, http.MethodPost, rosterPeople+"/bulk/activate", rosterIDs(idle), "limited") + if got := logs.matching("cabana: admin bulk action"); len(got) != 1 { + t.Fatalf("a refused action was logged as run: %q", got) + } + }) + + t.Run("T-12.1-09", func(t *testing.T) { + env, gdb := newRosterEnv(t) + rec := env.expect(t, http.StatusCreated, http.MethodPost, rosterPeople, `{"name":"Vic","notify":true,`+rosterPair+`}`, "bearer") + for _, name := range []string{"notify", "password"} { + if strings.Contains(rec.Body.String(), name) { + t.Fatalf("the create response carries the virtual field %s: %s", name, rec.Body.String()) + } + } + created, _ := rosterRecord(t, rec.Body.Bytes()).Data["id"].(float64) + stored := rosterLoad(t, gdb, uint(created)) + // The password column holds only what the hook derived; the submitted + // text never reached it through Fill. + if stored.Password != rosterHash("long-enough-1") { + t.Fatalf("a virtual value was bound to the model: password column = %q", stored.Password) + } + rec = env.expect(t, http.StatusOK, http.MethodGet, rosterPath(uint(created), ""), "", "bearer") + if strings.Contains(rec.Body.String(), "notify") || strings.Contains(rec.Body.String(), "password") { + t.Fatalf("the show response carries a virtual field: %s", rec.Body.String()) + } + }) + + t.Run("T-12.1-10", func(t *testing.T) { + env, gdb := newRosterEnv(t) + const plain = "s3cret-plain-text" + responses := map[string]string{} + rec := env.expect(t, http.StatusCreated, http.MethodPost, rosterPeople, fmt.Sprintf(`{"name":"Pat","password":%q,"password_confirmation":%q}`, plain, plain), "bearer") + responses["create"] = rec.Body.String() + created, _ := rosterRecord(t, rec.Body.Bytes()).Data["id"].(float64) + id := uint(created) + if rosterLoad(t, gdb, id).Password != rosterHash(plain) { + t.Fatal("the password did not reach the hook") + } + responses["show"] = env.expect(t, http.StatusOK, http.MethodGet, rosterPath(id, ""), "", "bearer").Body.String() + responses["list"] = env.expect(t, http.StatusOK, http.MethodGet, rosterPeople, "", "bearer").Body.String() + responses["update"] = env.expect(t, http.StatusOK, http.MethodPut, rosterPath(id, ""), fmt.Sprintf(`{"name":"Pat B","password":%q,"password_confirmation":%q}`, plain, plain), "bearer").Body.String() + responses["mismatch"] = env.expect(t, http.StatusUnprocessableEntity, http.MethodPut, rosterPath(id, ""), fmt.Sprintf(`{"password":%q,"password_confirmation":"other-enough-1"}`, plain), "bearer").Body.String() + for route, body := range responses { + if strings.Contains(body, plain) || strings.Contains(body, "sha256:") { + t.Fatalf("the %s response carries the password or its hash: %s", route, body) + } + if route != "mismatch" && strings.Contains(body, `"password`) { + t.Fatalf("the %s response carries a password key: %s", route, body) + } + } + }) + + t.Run("T-12.1-11", func(t *testing.T) { + // The same contract without WritableForeignKey: the field is read-only + // and a submitted id is not written. + env, gdb := newRosterEnvWith(t, func(p *rosterPlugin) { + p.relations = func(in []cabana.FieldRelationContract) []cabana.FieldRelationContract { + in[0].WritableForeignKey = false + return in + } + }) + team := rosterTeam{Tenant: "acme", Name: "Home"} + rosterSeed(t, gdb, &team) + person := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Ro", Active: true}) + _, raw := rosterFormSchema(t, env, "bearer") + if !strings.Contains(raw, `"name":"team","type":"relation","label":"Team","nameFrom":"name","emptyOption":"No team","readOnly":true}`) { + t.Fatalf("the team field is writable without the opt-in: %s", raw) + } + env.expect(t, http.StatusOK, http.MethodPut, rosterPath(person, ""), fmt.Sprintf(`{"team":%d,"organisation_id":%d}`, team.ID, team.ID), "bearer") + if stored := rosterLoad(t, gdb, person); stored.OrganisationID != nil { + t.Fatalf("a protected foreign key was written without the opt-in: %d", *stored.OrganisationID) + } + }) + + t.Run("T-12.1-12", func(t *testing.T) { + env, gdb := newRosterEnv(t) + staff, news := rosterTag{Name: "staff"}, rosterTag{Name: "news"} + rosterSeed(t, gdb, &staff) + rosterSeed(t, gdb, &news) + plain := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Plain", Active: true}) + member := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Member", Active: true}) + rosterSeed(t, gdb, &rosterPersonTag{PersonID: plain, TagID: news.ID}) + rosterSeed(t, gdb, &rosterPersonTag{PersonID: member, TagID: staff.ID}) + refused := func(method, rel, body string) { + t.Helper() + rec := env.expect(t, http.StatusForbidden, method, rel, body, "limited") + rosterErrorDetail(t, rec.Body.Bytes(), "forbidden", "tags", "You need an additional permission to change the staff tag.") + } + // Added on update, removed on update, added on create. + refused(http.MethodPut, rosterPath(plain, ""), fmt.Sprintf(`{"name":"Sneaky","tags":[%d,%d]}`, news.ID, staff.ID)) + refused(http.MethodPut, rosterPath(member, ""), `{"tags":[]}`) + refused(http.MethodPost, rosterPeople, fmt.Sprintf(`{"name":"Smuggled",%s,"tags":[%d]}`, rosterPair, staff.ID)) + if got := rosterPivot(t, gdb, plain); !reflect.DeepEqual(got, []uint{news.ID}) { + t.Fatalf("a refused save changed the pivot of the plain person: %v", got) + } + if got := rosterPivot(t, gdb, member); !reflect.DeepEqual(got, []uint{staff.ID}) { + t.Fatalf("a refused save changed the pivot of the member: %v", got) + } + if rosterLoad(t, gdb, plain).Name != "Plain" { + t.Fatal("a refused save wrote another field of the same body") + } + var smuggled int64 + if err := gdb.Unscoped().Model(&rosterPerson{}).Where("name = ?", "Smuggled").Count(&smuggled).Error; err != nil || smuggled != 0 { + t.Fatalf("a refused create left %d rows (%v)", smuggled, err) + } + }) + + t.Run("T-12.1-13", func(t *testing.T) { + env, gdb := newRosterEnv(t) + stored := `{"reports.export":1}` + id := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Perm", Active: true, Permissions: &stored}) + rec := env.expect(t, http.StatusUnprocessableEntity, http.MethodPut, rosterPath(id, ""), `{"permissions":{"admin.root":1,"reports.export":1}}`, "bearer") + rosterErrorDetail(t, rec.Body.Bytes(), "validation_failed", "permissions", "The permissions field contains an unknown permission.") + rec = env.expect(t, http.StatusUnprocessableEntity, http.MethodPut, rosterPath(id, ""), `{"permissions":{"posts.edit":2,"reports.export":1}}`, "bearer") + rosterErrorDetail(t, rec.Body.Bytes(), "validation_failed", "permissions", "The permissions field contains an invalid value.") + // The limited admin may not change the locked code, in either direction. + for _, body := range []string{`{"permissions":{"posts.edit":1}}`, `{"permissions":{"reports.export":-1}}`} { + rec = env.expect(t, http.StatusForbidden, http.MethodPut, rosterPath(id, ""), body, "limited") + rosterErrorDetail(t, rec.Body.Bytes(), "forbidden", "permissions", "You cannot change this permission.") + } + if got := rosterLoad(t, gdb, id).Permissions; got == nil || *got != stored { + t.Fatalf("a refused save changed the stored permissions: %v", got) + } + }) + + t.Run("T-12.1-14", func(t *testing.T) { + env, gdb := newRosterEnv(t) + ip := "203.0.113.7" + id := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Ada", Active: true, JoinedIP: &ip}) + env.expect(t, http.StatusOK, http.MethodPut, rosterPath(id, ""), `{"name":"Ada L","joined_ip":"198.51.100.1"}`, "bearer") + if got := rosterLoad(t, gdb, id); got.JoinedIP == nil || *got.JoinedIP != ip || got.Name != "Ada L" { + t.Fatalf("an update wrote the preview-only field: %+v", got.JoinedIP) + } + rec := env.expect(t, http.StatusCreated, http.MethodPost, rosterPeople, `{"name":"New","joined_ip":"198.51.100.2",`+rosterPair+`}`, "bearer") + created, _ := rosterRecord(t, rec.Body.Bytes()).Data["id"].(float64) + if got := rosterLoad(t, gdb, uint(created)); got.JoinedIP != nil { + t.Fatalf("a create wrote the preview-only field: %q", *got.JoinedIP) + } + }) + + t.Run("T-12.1-15", func(t *testing.T) { + // A status partial that carries active markup next to its callout. + const hostile = `
` + + `` + + `{{ trans .Data.Title }}` + + `a` + + `kept text
` + env, gdb := newRosterEnvWith(t, func(p *rosterPlugin) { + p.fsys = rosterTree(t, map[string]string{"controllers/people/_status.htm": hostile}) + }) + banned := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Bea", Active: true, Banned: true}) + rec := env.expect(t, http.StatusOK, http.MethodGet, fmt.Sprintf("%s/partials/status?id=%d", rosterPeople, banned), "", "bearer") + var view cabana.Envelope[cabana.PartialView] + if err := json.Unmarshal(rec.Body.Bytes(), &view); err != nil { + t.Fatalf("partial body %s: %v", rec.Body.String(), err) + } + if len(view.Data.Nodes) != 1 || view.Data.Nodes[0].Tag != "div" { + t.Fatalf("nodes = %+v", view.Data.Nodes) + } + root := view.Data.Nodes[0] + if !reflect.DeepEqual(root.Attrs, map[string]string{"class": "summer-callout", "data-tone": "danger"}) { + t.Fatalf("root attributes = %v, want only the allowlisted class and data-tone", root.Attrs) + } + var tags []string + for _, child := range root.Children { + tags = append(tags, child.Tag) + switch child.Tag { + case "a": + if !reflect.DeepEqual(child.Attrs, map[string]string{"title": "t"}) { + t.Fatalf("link attributes = %v, want no href and no handler", child.Attrs) + } + case "img": + if !reflect.DeepEqual(child.Attrs, map[string]string{"alt": "a"}) { + t.Fatalf("image attributes = %v, want no foreign src and no handler", child.Attrs) + } + } + } + // script and iframe are dropped with their content; the unknown element + // is unwrapped to its text. + if !reflect.DeepEqual(tags, []string{"a", "img", ""}) { + t.Fatalf("child tags = %q", tags) + } + for _, banned := range []string{"script", "iframe", "alert(1)", "onclick", "onerror", "onmouseover", "javascript:", "evil.example.test", "style", "custom-tag"} { + if strings.Contains(rec.Body.String(), banned) { + t.Fatalf("the partial response carries %q: %s", banned, rec.Body.String()) + } + } + }) +} diff --git a/scripts/check-phase12.1.sh b/scripts/check-phase12.1.sh new file mode 100755 index 0000000..9948aa7 --- /dev/null +++ b/scripts/check-phase12.1.sh @@ -0,0 +1,269 @@ +#!/usr/bin/env bash +# Phase 12.1 fail-closed gate (admin bulk and record actions, preview screen, +# row state, permission editor, form seams, and the user plugin's admin +# screens built on them). +# +# Every stage exits non-zero on a failing command, a go test run that fails, +# skips, matches zero tests or does not build, and a named security test that +# is missing, renamed or skipped. --self-test proves each detector fails +# closed on planted input. A stage that is not implemented refuses. +# +# Framework commands run in this repository. The plugin's tests run inside +# the application workspace named by PHASE121_APP (default: the sibling +# checkout next to this repository). Output about the application workspace +# has the application's name masked; set PHASE121_VERBOSE=1 to see it as it +# is while debugging. +# Run with FORCE_COLOR unset: bonfire's colour tests read it. +set -euo pipefail + +ROOT="${PHASE121_ROOT:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" +APP="${PHASE121_APP:-$ROOT/../fonoteka.go}" +# The user plugin inside the application workspace. +PLUGIN="./plugins/golem15/user" +APP_NAMES='fonoteka|p[lł]ytarium' + +# The named tests of the security stage, by prefix. Each prefix must match +# at least one top-level test that passes; any skip refuses. +SECURITY_CABANA=(TestPhase121Threats TestBulkAction TestRecordAction TestRowState TestForbidden + TestSoftDeletedRecord TestPreview TestPasswordField TestVirtualFields TestFormRules + TestPermissionEditor TestRelationLock TestWritableForeignKey TestInvisibleColumn + TestFilterOptionsController) +SECURITY_PLUGIN=(TestPhase121Threats TestAdminPrivilegedGroups TestAdminPrivilegedMember + TestAdminUserGroupsField TestAdminUserActions TestAdminUserForceDelete TestAdminUserPassword + TestAdminUserInvite TestAdminAvatarSharedWithAPI TestAdminGroups TestAdminOrganisations + TestAdminOrganisationMembers TestLastSeen) +SECURITY_PLUGIN_CLASSES=(TestMergedPermissions TestPermissionSetScan) + +STAGES=(self-test go security removal coverage spa openapi dist docs hygiene app evidence all) + +usage() { + cat >&2 <<'EOF' +usage: + check-phase12.1.sh --self-test + check-phase12.1.sh --go + check-phase12.1.sh --security + check-phase12.1.sh --removal + check-phase12.1.sh --coverage + check-phase12.1.sh --spa + check-phase12.1.sh --openapi + check-phase12.1.sh --dist + check-phase12.1.sh --docs + check-phase12.1.sh --hygiene + check-phase12.1.sh --app + check-phase12.1.sh --evidence + check-phase12.1.sh --all (every stage except --removal) + +environment: + PHASE121_APP the application workspace (default: the sibling checkout) + PHASE121_VERBOSE 1 shows application output without masking its name +EOF + exit 2 +} + +# mask hides the application's name in output about its workspace. +mask() { + if [[ "${PHASE121_VERBOSE:-}" == "1" ]]; then + cat + else + sed -E "s/($APP_NAMES)(\.go)?//gI" + fi +} + +# where DIR names a directory in output: the application workspace is never +# printed by its path. +where() { + if [[ "$1" == "$APP" ]]; then + echo "the application workspace" + else + echo "${1#"$ROOT"/}" + fi +} + +# detect reads go test -json. Exit 1 fail or build failure, 2 skip, 3 zero +# tests or "no tests to run", 4 non-JSON, 5 a required prefix has no passing +# top-level test. REQUIRE_PREFIXES lists the prefixes. +detect() { + python3 - "$1" <<'PY' +import json, os, sys +path = sys.argv[1] +prefixes = os.environ.get("REQUIRE_PREFIXES", "").split() +passed = set() +failed = [] +with open(path, encoding="utf-8", errors="replace") as fh: + for raw in fh: + line = raw.strip() + if not line.startswith("{"): + continue + try: + ev = json.loads(line) + except json.JSONDecodeError: + print("refuse: non-json test output", file=sys.stderr) + sys.exit(4) + action = ev.get("Action") + test = ev.get("Test") or "" + pkg = ev.get("Package") or ev.get("ImportPath") or "" + if action == "build-fail" or (action == "fail" and ev.get("FailedBuild")): + print(f"refuse: build failed {pkg}", file=sys.stderr) + sys.exit(1) + if action == "output" and "no tests to run" in (ev.get("Output") or ""): + print(f"refuse: no tests to run in {pkg}", file=sys.stderr) + sys.exit(3) + if action == "skip" and test: + print(f"refuse: skipped {pkg} {test}", file=sys.stderr) + sys.exit(2) + if action == "fail": + failed.append(f"{pkg} {test}".strip()) + if action == "pass" and test: + passed.add(test) +if failed: + print("refuse: failed " + ", ".join(failed), file=sys.stderr) + sys.exit(1) +if not passed: + print("refuse: zero tests", file=sys.stderr) + sys.exit(3) +top = {name for name in passed if "/" not in name} +missing = [p for p in prefixes if not any(name.startswith(p) for name in top)] +if missing: + print("refuse: missing named test: no passing test for " + ", ".join(missing), file=sys.stderr) + sys.exit(5) +PY +} + +# go_json DIR [go test args...] runs go test -json -count=1 through detect. +go_json() { + local dir="$1" + shift + local log err out rc=0 dc=0 + log="$(mktemp)" + err="$(mktemp)" + out="$(mktemp)" + (cd "$dir" && go test -json -count=1 "$@") >"$log" 2>"$err" || rc=$? + detect "$log" 2>"$out" || dc=$? + if [[ "$rc" -ne 0 || "$dc" -ne 0 ]]; then + { + cat "$err" || true + grep -v '^{' "$log" | tail -n 20 || true + python3 - "$log" <<'PY' || true +import json, sys +for raw in open(sys.argv[1], encoding="utf-8", errors="replace"): + try: + ev = json.loads(raw) + except ValueError: + continue + text = ev.get("Output") or "" + if ev.get("Action") == "output" and ("--- FAIL" in text or "_test.go:" in text or "panic:" in text): + sys.stdout.write(text) +PY + cat "$out" || true + echo "refuse: go test $* in $(where "$dir") (test=$rc detect=$dc)" + } 2>&1 | mask | tail -n 80 >&2 + rm -f "$log" "$err" "$out" + return 1 + fi + rm -f "$log" "$err" "$out" +} + +# named DIR PKG PREFIX... runs the tests matching the prefixes verbosely and +# requires a passing top-level test for each one. +named() { + local dir="$1" pkg="$2" + shift 2 + local regex + regex="^($( + IFS='|' + echo "$*" + ))" + REQUIRE_PREFIXES="$*" go_json "$dir" "$pkg" -v -run "$regex" +} + +expect_detect() { + local name="$1" want="$2" payload="$3" log dc=0 + log="$(mktemp)" + printf '%s\n' "$payload" >"$log" + detect "$log" 2>/dev/null || dc=$? + rm -f "$log" + if [[ "$dc" -ne "$want" ]]; then + echo "refuse: self-test $name: detector exit $dc, want $want" >&2 + return 1 + fi +} + +need_app() { + [[ -d "$APP" && -f "$APP/go.work" ]] || { + echo "refuse: the application workspace was not found (set PHASE121_APP)" >&2 + return 1 + } +} + +run_self_test() { + bash -n "${BASH_SOURCE[0]}" + expect_detect pass 0 '{"Action":"pass","Package":"p","Test":"TestPhase121Threats"}' + expect_detect fail 1 '{"Action":"pass","Package":"p","Test":"TestA"} +{"Action":"fail","Package":"p","Test":"TestPhase121Threats/T-12.1-28"}' + expect_detect package-fail 1 '{"Action":"pass","Package":"p","Test":"TestA"} +{"Action":"fail","Package":"p"}' + expect_detect build 1 '{"Action":"build-fail","ImportPath":"p"}' + expect_detect build-flag 1 '{"Action":"pass","Package":"q","Test":"TestA"} +{"Action":"fail","Package":"p","FailedBuild":"p"}' + expect_detect skip 2 '{"Action":"skip","Package":"p","Test":"TestPhase121Threats/T-12.1-38"}' + expect_detect zero 3 '{"Action":"pass","Package":"p"}' + expect_detect no-tests 3 '{"Action":"output","Package":"p","Output":"testing: warning: no tests to run\n"} +{"Action":"pass","Package":"p"}' + expect_detect nonjson 4 '{"Action":"pass",' + REQUIRE_PREFIXES="TestPhase121Threats TestAdminPrivilegedMember" expect_detect missing-named 5 \ + '{"Action":"pass","Package":"p","Test":"TestPhase121Threats"}' + REQUIRE_PREFIXES="TestAdminPrivilegedMember" expect_detect subtest-only 5 \ + '{"Action":"pass","Package":"p","Test":"TestOther/TestAdminPrivilegedMember"}' + REQUIRE_PREFIXES="TestPhase121Threats TestAdminPrivilegedMember" expect_detect named 0 \ + '{"Action":"pass","Package":"p","Test":"TestPhase121Threats"} +{"Action":"pass","Package":"p","Test":"TestAdminPrivilegedMember"}' + local stage + for stage in "${STAGES[@]}"; do + grep -q -- "^ --$stage)" "${BASH_SOURCE[0]}" || { + echo "refuse: missing mode --$stage" >&2 + return 1 + } + grep -q -- "check-phase12.1.sh --$stage" "${BASH_SOURCE[0]}" || { + echo "refuse: usage does not list --$stage" >&2 + return 1 + } + done + # The mask hides the application's name unless asked not to. + local masked + masked="$(printf 'ok \tgit.example.test/x/fonoteka.go/parity\n' | PHASE121_VERBOSE= mask)" + if grep -qiE "$APP_NAMES" <<<"$masked"; then + echo "refuse: self-test mask left the application's name: $masked" >&2 + return 1 + fi + echo "phase12.1 self-test passed" +} + +run_security() { + need_app + named "$ROOT" ./modules/cabana "${SECURITY_CABANA[@]}" + named "$APP" "$PLUGIN" "${SECURITY_PLUGIN[@]}" + named "$APP" "$PLUGIN/classes" "${SECURITY_PLUGIN_CLASSES[@]}" + echo "phase12.1 security passed" +} + +not_implemented() { + echo "refuse: stage --$1 is not implemented" >&2 + return 1 +} + +case "${1:-}" in + --self-test) run_self_test ;; + --go) not_implemented go ;; + --security) run_security ;; + --removal) not_implemented removal ;; + --coverage) not_implemented coverage ;; + --spa) not_implemented spa ;; + --openapi) not_implemented openapi ;; + --dist) not_implemented dist ;; + --docs) not_implemented docs ;; + --hygiene) not_implemented hygiene ;; + --app) not_implemented app ;; + --evidence) not_implemented evidence ;; + --all) not_implemented all ;; + *) usage ;; +esac