From 523d6a364921fe35664ef59f50678f491cbecb4f Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Mon, 7 Sep 2026 15:05:24 +0800 Subject: [PATCH] =?UTF-8?q?fix=F0=9F=90=9B:=20register=20the=20generator's?= =?UTF-8?q?=20writing=20endpoints=20only=20in=20a=20development=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of the code generator's endpoints do not read. /gen/toproject writes seven Go and Vue source files onto the host, one of them under the path gen.frontpath names; /gen/apitofile writes a migration; /gen/todb inserts menus and APIs. All three are GET, and all three are listed in CasbinExclude - which AuthCheckRole skips - so Enforce never runs for them. Any account that can log in could call them, on every deployment. They are now registered only where application.mode is dev or demo. dev is the shipped default and is where the generator is meant to be used. demo keeps them because demo mode already has a better answer than a 404: DemoEvn refuses these three by name and explains itself, which is what the demo is for. test and prod get nothing, and so does a process whose mode was never set. This does not make the endpoints safe where they exist; it stops them existing where nobody should be calling them. A host left on the shipped dev is still open, which is why the next commit says so at start-up. CasbinExclude is left alone on purpose. Taking the three off that list would make them require a permission no existing deployment has granted, so every non-admin user would start getting 403 from a tool that worked yesterday. That is a migration, not a guard, and it belongs with a release that can carry one. Four counter-proofs, each red on the test that names the behaviour and green everywhere else: a gate that always allows fails test/prod/unset only; a gate that always refuses fails dev/demo and takes TestEveryRouteDemoModeRefusesStillExists with it; moving a read-only route inside the gate fails the reading test; and spelling the condition at the registration site instead of calling the predicate fails the agreement test, which is what keeps that test from being a tautology. --- app/other/router/demo_guard_test.go | 17 ++++- app/other/router/gen_router.go | 42 +++++++++++- app/other/router/gen_router_test.go | 100 ++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 app/other/router/gen_router_test.go diff --git a/app/other/router/demo_guard_test.go b/app/other/router/demo_guard_test.go index cead37db..15a7d198 100644 --- a/app/other/router/demo_guard_test.go +++ b/app/other/router/demo_guard_test.go @@ -5,6 +5,7 @@ import ( "github.com/gin-gonic/gin" jwt "github.com/go-admin-team/go-admin-core/v2/jwtauth" + "github.com/go-admin-team/go-admin-core/v2/sdk/config" "go-admin/common/middleware" ) @@ -12,13 +13,23 @@ import ( // registeredRoutes builds the generator's routes on an engine of its own and // reports the patterns they were registered under. // +// The mode has to be given rather than inherited, because it now decides what +// gets registered: a test that leaves it at the zero value would be asking +// about a mode no deployment runs in, and would pass whether or not the gate +// works. The previous value is put back so the order of tests in this package +// cannot change their answers. +// // The JWT middleware is a zero value. MiddlewareFunc only closes over the // receiver and is never called here - no request is served, the engine is // asked what it has - so nothing dereferences it. -func registeredRoutes(t *testing.T) map[string]bool { +func registeredRoutes(t *testing.T, mode string) map[string]bool { t.Helper() gin.SetMode(gin.TestMode) + previous := config.ApplicationConfig.Mode + t.Cleanup(func() { config.ApplicationConfig.Mode = previous }) + config.ApplicationConfig.Mode = mode + r := gin.New() v1 := r.Group("/api/v1") sysNoCheckRoleRouter(v1, &jwt.GinJWTMiddleware{}) @@ -38,7 +49,7 @@ func registeredRoutes(t *testing.T) map[string]bool { // stops matching anything, demo mode silently starts serving it again, and // nothing else would say so. func TestEveryRouteDemoModeRefusesStillExists(t *testing.T) { - routes := registeredRoutes(t) + routes := registeredRoutes(t, "demo") for _, guarded := range middleware.DemoWriteRoutes() { if !routes[guarded] { t.Errorf("demo mode refuses %q, but no route is registered under that pattern - "+ @@ -64,7 +75,7 @@ func TestTheGeneratorsReadOnlyRoutesAreNotRefused(t *testing.T) { "/api/v1/db/tables/page", "/api/v1/db/columns/page", } { - if !registeredRoutes(t)[readOnly] { + if !registeredRoutes(t, "demo")[readOnly] { t.Fatalf("%s is not registered, so this test is asserting against nothing", readOnly) } if refused[readOnly] { diff --git a/app/other/router/gen_router.go b/app/other/router/gen_router.go index e3634403..6b8e39a4 100644 --- a/app/other/router/gen_router.go +++ b/app/other/router/gen_router.go @@ -3,10 +3,44 @@ package router import ( "github.com/gin-gonic/gin" jwt "github.com/go-admin-team/go-admin-core/v2/jwtauth" + "github.com/go-admin-team/go-admin-core/v2/sdk/config" + "go-admin/app/admin/apis" "go-admin/app/other/apis/tools" ) +// GenWriteRoutesEnabled reports whether the code generator's writing endpoints +// are registered in this process. +// +// Three of the generator's endpoints do not read. /gen/toproject writes seven +// Go and Vue source files onto this host, one of them under the path +// gen.frontpath names; /gen/apitofile writes a migration; /gen/todb inserts +// menus and APIs. All three are GET, all three are listed in CasbinExclude, +// and AuthCheckRole skips what is on that list - so Enforce never runs for +// them and any account that can log in may call them. That is a bargain a +// workstation can make and a deployment cannot. +// +// dev is the shipped default and is where the generator is meant to be used. +// demo keeps them because demo mode already has a better answer than a 404: +// DemoEvn refuses these three by name and explains itself, which is the thing +// the demo exists to show. test and prod get nothing. +// +// The mode is read once, while the routes are being built. Changing +// application.mode in a running process adds and removes nothing - a +// configuration reload rebuilds neither the engine nor its routes. +// +// core has constants for dev, test and prod but none for demo, which this +// repository spells as a literal in common/middleware/demo.go. Both are +// literals here so that the two read as one set rather than two conventions. +func GenWriteRoutesEnabled() bool { + switch config.ApplicationConfig.Mode { + case "dev", "demo": + return true + default: + return false + } +} + func init() { routerCheckRole = append(routerCheckRole, sysNoCheckRoleRouter, registerDBRouter, registerSysTableRouter) } @@ -22,9 +56,11 @@ func sysNoCheckRoleRouter(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMiddlew { gen := tools.Gen{} r.GET("/gen/preview/:tableId", gen.Preview) - r.GET("/gen/toproject/:tableId", gen.GenCode) - r.GET("/gen/apitofile/:tableId", gen.GenApiToFile) - r.GET("/gen/todb/:tableId", gen.GenMenuAndApi) + if GenWriteRoutesEnabled() { + r.GET("/gen/toproject/:tableId", gen.GenCode) + r.GET("/gen/apitofile/:tableId", gen.GenApiToFile) + r.GET("/gen/todb/:tableId", gen.GenMenuAndApi) + } sysTable := tools.SysTable{} r.GET("/gen/tabletree", sysTable.GetSysTablesTree) } diff --git a/app/other/router/gen_router_test.go b/app/other/router/gen_router_test.go new file mode 100644 index 00000000..01452766 --- /dev/null +++ b/app/other/router/gen_router_test.go @@ -0,0 +1,100 @@ +package router + +import ( + "testing" + + "github.com/go-admin-team/go-admin-core/v2/sdk/config" +) + +// genWritingRoutes are the three that do not read. They write Go and Vue +// source onto the host, a migration, and rows in sys_menu. +var genWritingRoutes = []string{ + "/api/v1/gen/toproject/:tableId", + "/api/v1/gen/apitofile/:tableId", + "/api/v1/gen/todb/:tableId", +} + +// genReadingRoutes are the rest of the generator's surface. Gating the three +// above must not cost any of these: a deployment that cannot list its tables +// or preview a template has lost the feature, not secured it. +var genReadingRoutes = []string{ + "/api/v1/gen/preview/:tableId", + "/api/v1/gen/tabletree", + "/api/v1/db/tables/page", + "/api/v1/db/columns/page", +} + +// The endpoints that write are registered where the mode says development and +// nowhere else. +// +// They are in CasbinExclude, so Enforce never runs for them and any account +// that can log in may call them. dev is the shipped default and is where the +// generator is meant to be used; demo keeps them because DemoEvn refuses these +// three by name and saying so is the thing the demo is for. Everything else, +// including the empty mode a process gets when nothing set one, is refused by +// not existing. +func TestGeneratorWritingRoutesExistOnlyWhereTheModeAllowsIt(t *testing.T) { + for _, tc := range []struct { + mode string + expected bool + why string + }{ + {"dev", true, "the shipped default, and where the generator is used"}, + {"demo", true, "registered so demo mode can refuse them by name"}, + {"test", false, "a deployment, however much it is called a test"}, + {"prod", false, "a deployment"}, + {"", false, "no mode configured is not a reason to trust the caller"}, + } { + t.Run("mode="+tc.mode, func(t *testing.T) { + routes := registeredRoutes(t, tc.mode) + for _, writing := range genWritingRoutes { + if got := routes[writing]; got != tc.expected { + t.Errorf("mode %q: %s registered = %v, want %v (%s)", + tc.mode, writing, got, tc.expected, tc.why) + } + } + }) + } +} + +// The other direction. Refusing too much is as much of a defect as refusing +// too little, and the read-only half of the generator is what a demo shows. +func TestGeneratorReadingRoutesExistInEveryMode(t *testing.T) { + for _, mode := range []string{"dev", "demo", "test", "prod", ""} { + t.Run("mode="+mode, func(t *testing.T) { + routes := registeredRoutes(t, mode) + for _, reading := range genReadingRoutes { + if !routes[reading] { + t.Errorf("mode %q: %s is not registered - the gate took a route that only reads", + mode, reading) + } + } + }) + } +} + +// GenWriteRoutesEnabled is what cmd/api reads to decide whether to warn at +// start-up. If it and the registration ever disagree, the log says one thing +// and the engine does another, so they are checked against each other rather +// than each against a list. +func TestGenWriteRoutesEnabledAgreesWithWhatWasRegistered(t *testing.T) { + for _, mode := range []string{"dev", "demo", "test", "prod", ""} { + t.Run("mode="+mode, func(t *testing.T) { + routes := registeredRoutes(t, mode) + + // registeredRoutes puts the mode back before it returns, so it + // has to be set again to ask the predicate about the same mode + // the engine was built under. + previous := config.ApplicationConfig.Mode + t.Cleanup(func() { config.ApplicationConfig.Mode = previous }) + config.ApplicationConfig.Mode = mode + + claimed := GenWriteRoutesEnabled() + actual := routes["/api/v1/gen/todb/:tableId"] + if claimed != actual { + t.Errorf("mode %q: GenWriteRoutesEnabled() = %v but the route was registered = %v", + mode, claimed, actual) + } + }) + } +}