diff --git a/app/other/apis/tools/gen.go b/app/other/apis/tools/gen.go index ad64b1dc..47b0cf8c 100644 --- a/app/other/apis/tools/gen.go +++ b/app/other/apis/tools/gen.go @@ -241,6 +241,12 @@ func (e Gen) NOActionsGen(c *gin.Context, tab tools.SysTables) bool { e.Error(500, err, err.Error()) return false } + // Checked again here, not only on save: a configuration saved before the + // save-time check existed is still in sys_tables. + if err := validateGenPathFields(tab); err != nil { + e.Error(500, err, err.Error()) + return false + } // R2: see the matching call and comment in Preview above. applyInferredColumnWidths(tab.Columns) diff --git a/app/other/apis/tools/gen_paths_test.go b/app/other/apis/tools/gen_paths_test.go index f5db9fc1..4eef5074 100644 --- a/app/other/apis/tools/gen_paths_test.go +++ b/app/other/apis/tools/gen_paths_test.go @@ -1,13 +1,18 @@ package tools import ( + "encoding/json" "io/fs" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" "testing" "github.com/gin-gonic/gin" + + "go-admin/app/other/models/tools" ) // The generator builds the paths it writes to from three fields of a table's @@ -57,6 +62,40 @@ func TestNOActionsGenWritesEveryFileInsideItsRoots(t *testing.T) { } } +// A configuration already in sys_tables reaches NOActionsGen as it was saved, +// so this is also the case of a row saved before the save-time check existed. +func TestNOActionsGenRefusesAPathFieldThatLeavesItsRoot(t *testing.T) { + cases := map[string]func(*tools.SysTables){ + "packageName": func(tab *tools.SysTables) { tab.PackageName = "../../escaped" }, + "tableName": func(tab *tools.SysTables) { tab.TBName = "../../../escaped" }, + "businessName": func(tab *tools.SysTables) { tab.BusinessName = "../../../../escaped" }, + "absolute": func(tab *tools.SysTables) { tab.PackageName = "/escaped" }, + } + for name, spoil := range cases { + t.Run(name, func(t *testing.T) { + dir := genWorkspace(t) + tab := genTable() + spoil(&tab) + + var ok bool + bodies := runGen(t, nil, func(c *gin.Context) { ok = Gen{}.NOActionsGen(c, tab) }, nil) + + if ok || len(bodies) != 1 || bodies[0].Code != 500 || !strings.Contains(bodies[0].Msg, "不合法") { + t.Fatalf("ok=%v, responses %+v; want false and one 500 naming the invalid field", ok, bodies) + } + if files := writtenFiles(t, dir); len(files) > 0 { + t.Errorf("wrote %v inside the workspace", files) + } + // Where each of the paths above would have landed. + for _, p := range []string{filepath.Join(dir, "..", "escaped"), filepath.Join(filepath.Dir(dir), "..", "escaped"), "/escaped"} { + if _, err := os.Stat(p); err == nil { + t.Errorf("%s exists after a refused generation", p) + } + } + }) + } +} + // A symlink under a root that points out of it is the case a name check // cannot see: every component of the name is legal. os.Root refuses to follow // it. The file planted where the write would land must survive unchanged. @@ -95,3 +134,88 @@ func TestNOActionsGenDoesNotFollowASymlinkOutOfItsRoot(t *testing.T) { }) } } + +// putTable saves a table's configuration the way the config page does. +func putTable(t *testing.T, tab tools.SysTables) genBody { + t.Helper() + body, _ := json.Marshal(tab) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest(http.MethodPut, "/", strings.NewReader(string(body))) + c.Request.Header.Set("Content-Type", "application/json") + c.Set("db", genDB(t)) + SysTable{}.Update(c) + var res genBody + if err := json.Unmarshal(w.Body.Bytes(), &res); err != nil { + t.Fatalf("decoding %q: %v", w.Body.String(), err) + } + return res +} + +func TestSavingAConfigurationRefusesAPathField(t *testing.T) { + db := genDB(t) + stored := genTable() + created, err := stored.Create(db) + if err != nil { + t.Fatal(err) + } + + edited := created + edited.PackageName = "../escaped" + if res := putTable(t, edited); res.Code != 500 || !strings.Contains(res.Msg, "packageName") { + t.Fatalf("response %+v; want 500 naming packageName", res) + } + + var row tools.SysTables + if err := db.First(&row, created.TableId).Error; err != nil { + t.Fatal(err) + } + if row.PackageName != "admin" { + t.Errorf("package_name = %q after a refused save, want it unchanged", row.PackageName) + } +} + +// What the config page and the importer produce has to keep passing. +func TestValidateGenPathFieldsAcceptsWhatTheConfigPageAccepts(t *testing.T) { + for _, tab := range []tools.SysTables{ + {PackageName: "admin", TBName: "sys_user", BusinessName: "sysUser"}, + {PackageName: "shop2", TBName: "order2", BusinessName: "order2"}, + {PackageName: "x", TBName: "T_Mixed_1", BusinessName: "tMixed1"}, + } { + if err := validateGenPathFields(tab); err != nil { + t.Errorf("%+v refused: %v", tab, err) + } + } +} + +// The importer reads table names from the database, which accepts names no +// file can carry. A list holding one is refused whole: the valid table +// beside it is not imported either. +func TestImportRefusesATableNameThatCannotBeAFileName(t *testing.T) { + g := newGenEnv(t) + const good, bad = "gpa_ok", "gpa-dash" + // dropFixture does not quote the name, and the point of bad is that it + // needs quoting, so the rows and tables are removed here. + drop := func() { + g.db.Unscoped().Where("table_name IN ?", []string{good, bad}).Delete(&tools.SysTables{}) + for _, table := range []string{good, bad} { + g.db.Exec("DROP TABLE IF EXISTS `" + table + "`") + } + } + drop() + t.Cleanup(drop) + for _, table := range []string{good, bad} { + if err := g.db.Exec("CREATE TABLE `" + table + "` (id int NOT NULL AUTO_INCREMENT PRIMARY KEY," + auditColumns + ")").Error; err != nil { + t.Fatalf("creating %s: %v", table, err) + } + } + + if code := callHandler(t, g.db, SysTable{}.Insert, "/?tables="+good+","+bad, nil); code == http.StatusOK { + t.Fatal("the import succeeded") + } + var n int64 + g.db.Model(&tools.SysTables{}).Where("table_name IN ?", []string{good, bad}).Count(&n) + if n != 0 { + t.Errorf("%d table(s) imported from a refused list, want 0", n) + } +} diff --git a/app/other/apis/tools/sys_tables.go b/app/other/apis/tools/sys_tables.go index e8c84fea..8165f4ce 100644 --- a/app/other/apis/tools/sys_tables.go +++ b/app/other/apis/tools/sys_tables.go @@ -172,17 +172,26 @@ func (e SysTable) Insert(c *gin.Context) { return } + // Every table is read and checked before any is saved, so a list with + // one table whose name cannot become a file name is refused whole + // instead of importing the tables ahead of it. + tables := make([]tools.SysTables, 0, len(tablesList)) for i := 0; i < len(tablesList); i++ { - data, err := genTableInit(db, tablesList, i, c) if err != nil { log.Errorf("genTableInit error, %s", err.Error()) e.Error(500, err, "") return } - - _, err = data.Create(db) - if err != nil { + if err = validateGenPathFields(data); err != nil { + log.Errorf("validate table error, %s", err.Error()) + e.Error(500, err, err.Error()) + return + } + tables = append(tables, data) + } + for i := range tables { + if _, err = tables[i].Create(db); err != nil { log.Errorf("Create error, %s", err.Error()) e.Error(500, err, "") return @@ -377,6 +386,11 @@ func (e SysTable) Update(c *gin.Context) { e.Error(500, err, err.Error()) return } + if err = validateGenPathFields(data); err != nil { + log.Errorf("validate table error, %s", err.Error()) + e.Error(500, err, err.Error()) + return + } if err = validateBusinessNameUnique(db, data.PackageName, data.BusinessName, data.TableId); err != nil { log.Errorf("validate businessName error, %s", err.Error()) e.Error(500, err, err.Error()) diff --git a/app/other/apis/tools/sys_tables_validate.go b/app/other/apis/tools/sys_tables_validate.go index 067f6aaa..ba6576c4 100644 --- a/app/other/apis/tools/sys_tables_validate.go +++ b/app/other/apis/tools/sys_tables_validate.go @@ -110,3 +110,44 @@ func validateBusinessNameUnique(db *gorm.DB, packageName, businessName string, t } return nil } + +// The three fields gen.go joins into the paths it writes to. Each is checked +// against what it has to be where it lands, not against one shared pattern: +// +// - packageName names a Go package and the app/{packageName} directory. +// Lowercase letters and digits, as a Go package name should be; no +// hyphen, which Go rejects, and no underscore, which genInfoForm.vue +// already refuses. +// - tableName is the imported table's own name, and becomes a .go file name +// and, with "_" turned into "-", a .ts/.vue one. Letters, digits and +// underscores: what a table the importer can read is normally called, and +// nothing that can step out of a directory. +// - businessName is a JavaScript identifier and a .ts file name. It starts +// with a lowercase letter, as genInfoForm.vue requires, but may carry +// digits, which a name derived from a table such as order2 does. +// +// None of the three allows a dot or a path separator, so none can name a +// parent directory or an absolute path. The rules are no stricter than the +// config page's own, so nothing it accepts is refused here. +var ( + packageNamePattern = regexp.MustCompile(`^[a-z][a-z0-9]*$`) + tableNamePattern = regexp.MustCompile(`^[A-Za-z0-9_]+$`) + businessNamePattern = regexp.MustCompile(`^[a-z][A-Za-z0-9]*$`) +) + +// validateGenPathFields checks the fields gen.go builds file paths from. It +// runs where a configuration is saved and again before files are written, so +// a row saved before this check existed is refused at generation rather than +// trusted because it is already in the database. +func validateGenPathFields(tab tools.SysTables) error { + if !packageNamePattern.MatchString(tab.PackageName) { + return fmt.Errorf("packageName=%q 不合法：只能包含小写字母和数字，且以字母开头", tab.PackageName) + } + if !tableNamePattern.MatchString(tab.TBName) { + return fmt.Errorf("tableName=%q 不合法：只能包含字母、数字和下划线", tab.TBName) + } + if !businessNamePattern.MatchString(tab.BusinessName) { + return fmt.Errorf("businessName=%q 不合法：只能包含字母和数字，且以小写字母开头", tab.BusinessName) + } + return nil +}