docs📝(checksilent): describe the two passes the code actually makes

The comment claimed bindings were collected as the body was walked so that a
registration only saw definitions above it. That is the single-pass design this
started as. The code does two passes - one to collect, one to report - and a
registration therefore sees every binding in the function.

That is the point rather than an accident: a `.Use` written below a route is
still part of the chain, because the chain is assembled before anything is
served. The price is that a name reused for two different things in one
function resolves to the last assignment, which the comment now says instead of
promising an ordering the code does not keep.

app/admin/apis/sys_user.go

@@ -444,7 +444,6 @@ func (e SysUser) GetInfo(c *gin.Context) {
 		e.Error(500, err, err.Error())
 		return
 	}
-	p := actions.GetPermissionFromContext(c)
 	var roles = make([]string, 1)
 	roles[0] = user.GetRoleName(c)
 	var permissions = make([]string, 1)
@@ -464,7 +463,14 @@ func (e SysUser) GetInfo(c *gin.Context) {
 	}
 	sysUser := models.SysUser{}
 	req.Id = user.GetUserId(c)
-	err = s.Get(&req, p, &sysUser)
+	// Unscoped on purpose: the id is the caller's own, taken from the token.
+	// This used to go through Get with whatever GetPermissionFromContext
+	// returned - and this route installs no PermissionAction, so that was the
+	// zero value. An unset scope is not a recognised one, so once unknown
+	// scopes started failing closed rather than silently matching everything,
+	// every login on a deployment with enabledp: true ended here with a 401
+	// and the browser went straight back to the login page.
+	err = s.GetSelf(&req, &sysUser)
 	if err != nil {
 		e.Error(http.StatusUnauthorized, err, "登录失败")
 		return

app/admin/router/sys_api.go

@@ -5,6 +5,7 @@ import (
 	jwt "github.com/go-admin-team/go-admin-core/v2/jwtauth"
 
 	"go-admin/app/admin/apis"
+	"go-admin/common/actions"
 	"go-admin/common/middleware"
 )
 
@@ -15,7 +16,10 @@ func init() {
 // registerSysApiRouter
 func registerSysApiRouter(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMiddleware) {
 	api := apis.SysApi{}
-	r := v1.Group("/sys-api").Use(authMiddleware.MiddlewareFunc()).Use(middleware.AuthCheckRole())
+	// PermissionAction is not optional here: all three handlers below read the
+	// data permission out of the context, and without it they read the zero
+	// value - an unset scope, which Permission now fails closed on.
+	r := v1.Group("/sys-api").Use(authMiddleware.MiddlewareFunc()).Use(middleware.AuthCheckRole()).Use(actions.PermissionAction())
 	{
 		r.GET("", api.GetPage)
 		r.GET("/:id", api.Get)

app/admin/service/sys_user.go

@@ -38,6 +38,30 @@ func (e *SysUser) GetPage(c *dto.SysUserGetPageReq, p *actions.DataPermission, l
 	return nil
 }
 
+// GetSelf 获取调用者自己的 SysUser 对象，不套数据权限
+//
+// The data scope answers "whose rows may this user see"; the caller here is
+// reading their own, and the id comes from the token, so there is nothing left
+// for a scope to restrict. Applying one is not a stricter version of this
+// query - it is a broken one. DataScopeSelf matches on create_by, and a user
+// account is created by whoever added it, so a scoped self-read would fail for
+// every user who did not create their own account.
+//
+// GetProfile has always read the same row this way, with no scope at all.
+func (e *SysUser) GetSelf(d *dto.SysUserById, model *models.SysUser) error {
+	err := e.Orm.First(model, d.GetId()).Error
+	if err != nil && errors.Is(err, gorm.ErrRecordNotFound) {
+		err = errors.New("查看对象不存在或无权查看")
+		e.Log.Errorf("db error: %s", err)
+		return err
+	}
+	if err != nil {
+		e.Log.Errorf("db error: %s", err)
+		return err
+	}
+	return nil
+}
+
 // Get 获取SysUser对象
 func (e *SysUser) Get(d *dto.SysUserById, p *actions.DataPermission, model *models.SysUser) error {
 	var data models.SysUser

cmd/api/server.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
+	"syscall"
 	"time"
 
 	"github.com/gin-gonic/gin"
@@ -85,8 +86,8 @@ func run() error {
 	runStartupHooks()
 
 	srv := &http.Server{
-		Addr:    fmt.Sprintf("%s:%d", config.ApplicationConfig.Host, config.ApplicationConfig.Port),
-		Handler: sdk.Runtime.GetEngine(),
+		Addr:         fmt.Sprintf("%s:%d", config.ApplicationConfig.Host, config.ApplicationConfig.Port),
+		Handler:      sdk.Runtime.GetEngine(),
 		ReadTimeout:  time.Duration(config.ApplicationConfig.ReadTimeout) * time.Second,
 		WriteTimeout: time.Duration(config.ApplicationConfig.WriterTimeout) * time.Second,
 	}
@@ -114,6 +115,13 @@ func run() error {
 		}
 	}
 
+	// Armed before the server starts serving, and well before the readiness
+	// banner: a signal arriving between "the process is up" and "the process
+	// is listening for signals" reaches the default handler and kills it
+	// without any of the shutdown below. That window is the whole reason
+	// arming is separate from waiting.
+	quit, disarmStopSignals := armStopSignals()
+
 	go func() {
 		// 服务连接
 		if config.SslConfig.Enable {
@@ -135,23 +143,59 @@ func run() error {
 	fmt.Printf("-  Local:   http://localhost:%d/swagger/admin/index.html \r\n", config.ApplicationConfig.Port)
 	fmt.Printf("-  Network: %s://%s:%d/swagger/admin/index.html \r\n", "http", pkg.GetLocalHost(), config.ApplicationConfig.Port)
 	fmt.Printf("%s Enter Control + C Shutdown Server \r\n", pkg.GetCurrentTimeStr())
-	// 等待中断信号以优雅地关闭服务器（设置 5 秒的超时时间）
-	quit := make(chan os.Signal, 1)
-	signal.Notify(quit, os.Interrupt)
+
 	<-quit
+	// Restored here, not deferred: from this point a second signal must reach
+	// the default handler, so a shutdown that hangs can still be interrupted.
+	disarmStopSignals()
 
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-	defer cancel()
 	log.Info("Shutdown Server ... ")
-
-	if err := srv.Shutdown(ctx); err != nil {
-		log.Fatal("Server Shutdown:", err)
+	if err := shutdownServer(srv, shutdownTimeout); err != nil {
+		// Not log.Fatal: that is an unconditional os.Exit(1), and Shutdown
+		// reports an error exactly when connections were still in flight -
+		// which is when the cleanup that follows matters most.
+		log.Error("Server Shutdown: ", err)
 	}
 	log.Info("Server exiting")
 
 	return nil
 }
 
+// shutdownTimeout is how long Shutdown waits for in-flight requests. It plus
+// whatever cleanup follows has to stay inside the orchestrator's grace period
+// - `docker stop` allows 10s by default before it sends SIGKILL.
+const shutdownTimeout = 5 * time.Second
+
+// armStopSignals registers for the stop signals and returns the channel they
+// arrive on together with the function that restores the default disposition.
+//
+// SIGTERM is what actually arrives in production: `docker stop`, a Kubernetes
+// pod deletion and `systemctl stop` all send it, and Go terminates the process
+// immediately for a signal nobody listens for. Registering only os.Interrupt
+// meant every graceful shutdown below the wait was dead code outside a
+// terminal.
+//
+// Registering is separate from waiting so a caller can arm before it announces
+// that it is ready: a signal that arrives between the two is delivered to the
+// default handler, which for both of these means the process dies without
+// running any of this.
+func armStopSignals() (<-chan os.Signal, func()) {
+	quit := make(chan os.Signal, 1)
+	signal.Notify(quit, os.Interrupt, syscall.SIGTERM)
+	return quit, func() { signal.Stop(quit) }
+}
+
+// shutdownServer stops srv, giving in-flight requests up to timeout to finish.
+//
+// It returns the error instead of exiting on it. A caller that exits here skips
+// its own cleanup, and Shutdown fails precisely when there was something left
+// to clean up after.
+func shutdownServer(srv *http.Server, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+	return srv.Shutdown(ctx)
+}
+
 // runStartupHooks runs the router registries and then the before callbacks.
 //
 // The package-level slice runs first and in its existing order, so a fork that

cmd/api/signal_test.go

@@ -0,0 +1,292 @@
+package api
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+)
+
+// The signal path cannot be exercised in-process: delivering a signal to the
+// test binary would race with the test framework, and the disposition changes
+// are global. So the test re-executes itself as a child, and the child runs the
+// same armStopSignals / shutdownServer the server does.
+//
+// The child deliberately serves an empty http.Server rather than the real one:
+// this repository's CI has no database (.github/workflows/go.yml runs neither
+// MySQL nor a sqlite-tagged build), and none of what is under test needs one.
+const (
+	childEnv       = "GO_ADMIN_SIGNAL_CHILD"
+	childStuckEnv  = "GO_ADMIN_SIGNAL_CHILD_STUCK"
+	childHangConn  = "GO_ADMIN_SIGNAL_CHILD_HANGCONN"
+	markerReady    = "CHILD-READY"
+	markerSignal   = "CHILD-SIGNAL"
+	markerShutdown = "CHILD-SHUTDOWN-OK"
+	markerExiting  = "CHILD-EXITING"
+)
+
+// TestSignalChild is the child process. It is skipped in a normal run.
+func TestSignalChild(t *testing.T) {
+	if os.Getenv(childEnv) != "1" {
+		t.Skip("child process entry point")
+	}
+
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		fmt.Println("listen:", err)
+		os.Exit(3)
+	}
+	// accepted fires once the server has taken a connection off the listener.
+	// Dialling is not enough: Shutdown only waits for connections the server
+	// has already accepted, so calling it between the dial and the accept
+	// finds nothing to wait for and returns immediately.
+	accepted := make(chan struct{}, 1)
+	srv := &http.Server{
+		Handler: http.NewServeMux(),
+		ConnState: func(_ net.Conn, state http.ConnState) {
+			if state == http.StateNew {
+				select {
+				case accepted <- struct{}{}:
+				default:
+				}
+			}
+		},
+	}
+	go func() { _ = srv.Serve(ln) }()
+
+	// Arm before announcing readiness. Doing it the other way round leaves a
+	// window in which the parent's signal reaches the default handler and
+	// kills the child before any of this runs - which is exactly the failure
+	// this whole change is about, so the test must not reproduce it by
+	// accident.
+	quit, disarm := armStopSignals()
+
+	fmt.Println(markerReady)
+	os.Stdout.Sync()
+
+	sig := <-quit
+	disarm()
+	fmt.Println(markerSignal, sig)
+	os.Stdout.Sync()
+
+	if os.Getenv(childStuckEnv) == "1" {
+		// Stand in for a cleanup hook that never finishes. The point of
+		// restoring the signal disposition is that a second signal still
+		// reaches the default handler and kills this.
+		time.Sleep(2 * time.Minute)
+	}
+
+	timeout := shutdownTimeout
+	if os.Getenv(childHangConn) == "1" {
+		// Dialled here, not at start-up. net/http stops counting a StateNew
+		// connection against Shutdown once it is more than five seconds old,
+		// so a connection opened before the wait would age out on a slow CI
+		// run and Shutdown would succeed - leaving the test asserting nothing.
+		c, err := net.Dial("tcp", ln.Addr().String())
+		if err != nil {
+			fmt.Println("dial:", err)
+			os.Exit(5)
+		}
+		defer func() { _ = c.Close() }()
+
+		// And wait for the accept, for the opposite reason: an unaccepted
+		// connection is not one Shutdown waits for either.
+		select {
+		case <-accepted:
+		case <-time.After(10 * time.Second):
+			fmt.Println("the server never accepted the stalling connection")
+			os.Exit(6)
+		}
+
+		// A connection that has sent nothing keeps Shutdown busy: net/http
+		// only treats a StateNew connection as idle once it is more than five
+		// seconds old. A short budget makes the timeout deterministic without
+		// waiting out the real one.
+		timeout = 300 * time.Millisecond
+	}
+
+	if err := shutdownServer(srv, timeout); err != nil {
+		// Deliberately not fatal, and deliberately not a bare return: the
+		// point is that whatever follows still runs.
+		fmt.Println("shutdown error:", err)
+	} else {
+		fmt.Println(markerShutdown)
+	}
+	fmt.Println(markerExiting)
+	os.Stdout.Sync()
+}
+
+func startChild(t *testing.T, stuck bool, extraEnv ...string) (*exec.Cmd, *os.File, chan string) {
+	t.Helper()
+
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("pipe: %v", err)
+	}
+	cmd := exec.Command(os.Args[0], "-test.run=TestSignalChild", "-test.v")
+	cmd.Env = append(os.Environ(), childEnv+"=1")
+	if stuck {
+		cmd.Env = append(cmd.Env, childStuckEnv+"=1")
+	}
+	cmd.Env = append(cmd.Env, extraEnv...)
+	cmd.Stdout = w
+	cmd.Stderr = w
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start child: %v", err)
+	}
+	_ = w.Close()
+
+	lines := make(chan string, 64)
+	go func() {
+		defer close(lines)
+		buf := make([]byte, 4096)
+		var acc strings.Builder
+		for {
+			n, err := r.Read(buf)
+			if n > 0 {
+				acc.Write(buf[:n])
+				for {
+					s := acc.String()
+					i := strings.IndexByte(s, '\n')
+					if i < 0 {
+						break
+					}
+					lines <- s[:i]
+					acc.Reset()
+					acc.WriteString(s[i+1:])
+				}
+			}
+			if err != nil {
+				if acc.Len() > 0 {
+					lines <- acc.String()
+				}
+				return
+			}
+		}
+	}()
+
+	t.Cleanup(func() {
+		_ = cmd.Process.Kill()
+		_, _ = cmd.Process.Wait()
+		_ = r.Close()
+	})
+	return cmd, r, lines
+}
+
+// await drains lines until one contains want, or the deadline passes. It
+// returns everything it saw, so a failure says what the child actually did.
+func await(t *testing.T, lines chan string, want string, d time.Duration) []string {
+	t.Helper()
+	var seen []string
+	deadline := time.After(d)
+	for {
+		select {
+		case l, ok := <-lines:
+			if !ok {
+				t.Fatalf("child output ended before %q; saw:\n%s", want, strings.Join(seen, "\n"))
+			}
+			seen = append(seen, l)
+			if strings.Contains(l, want) {
+				return seen
+			}
+		case <-deadline:
+			t.Fatalf("timed out waiting for %q; saw:\n%s", want, strings.Join(seen, "\n"))
+		}
+	}
+}
+
+// Acceptance 19. Registering only os.Interrupt meant SIGTERM - the signal
+// `docker stop`, Kubernetes and systemd all send - terminated the process
+// before any of the shutdown path ran. Both must now reach it.
+func TestBothSignalsRunTheShutdownPath(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		sig  syscall.Signal
+	}{
+		{"SIGINT", syscall.SIGINT},
+		{"SIGTERM", syscall.SIGTERM},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cmd, _, lines := startChild(t, false)
+			await(t, lines, markerReady, 30*time.Second)
+
+			if err := cmd.Process.Signal(tc.sig); err != nil {
+				t.Fatalf("signal: %v", err)
+			}
+
+			await(t, lines, markerSignal, 10*time.Second)
+			await(t, lines, markerShutdown, 10*time.Second)
+			await(t, lines, markerExiting, 10*time.Second)
+
+			if err := cmd.Wait(); err != nil {
+				t.Fatalf("child exited with %v, want a clean exit", err)
+			}
+		})
+	}
+}
+
+// Acceptance 20. quit is a buffered channel and signal.Notify stays armed, so
+// without restoring the disposition a second signal only refills the buffer:
+// once SIGTERM is registered, a shutdown that hangs could not be interrupted by
+// anything short of SIGKILL.
+func TestASecondSignalStillKillsAStuckShutdown(t *testing.T) {
+	cmd, _, lines := startChild(t, true)
+	await(t, lines, markerReady, 30*time.Second)
+
+	if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
+		t.Fatalf("first signal: %v", err)
+	}
+	await(t, lines, markerSignal, 10*time.Second)
+
+	// The child is now inside a cleanup that will not finish on its own.
+	if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
+		t.Fatalf("second signal: %v", err)
+	}
+
+	done := make(chan error, 1)
+	go func() { done <- cmd.Wait() }()
+
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("child exited cleanly; it was supposed to be killed by the second signal")
+		}
+	case <-time.After(15 * time.Second):
+		t.Fatal("the second signal did not kill a stuck shutdown - the escape hatch is gone")
+	}
+}
+
+// Acceptance 21. srv.Shutdown reports an error exactly when connections were
+// still in flight, and the old code answered that with log.Fatal - an
+// unconditional os.Exit(1). Everything after it, which is where the cleanup
+// hooks will hang, never ran. A failed Shutdown must not end the process.
+func TestShutdownTimeoutDoesNotStopWhatFollows(t *testing.T) {
+	cmd, _, lines := startChild(t, false, childHangConn+"=1")
+	await(t, lines, markerReady, 30*time.Second)
+
+	if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
+		t.Fatalf("signal: %v", err)
+	}
+	await(t, lines, markerSignal, 10*time.Second)
+
+	seen := await(t, lines, markerExiting, 20*time.Second)
+
+	var timedOut bool
+	for _, l := range seen {
+		if strings.Contains(l, "shutdown error:") {
+			timedOut = true
+		}
+	}
+	if !timedOut {
+		t.Fatalf("Shutdown did not time out, so this test proves nothing; saw:\n%s",
+			strings.Join(seen, "\n"))
+	}
+	if err := cmd.Wait(); err != nil {
+		t.Fatalf("child exited with %v after a failed Shutdown, want a clean exit", err)
+	}
+}

tools/checksilent/checks.go

@@ -20,6 +20,7 @@ const (
 	checkMenuIDConflict = "menu-id-collision"
 	checkImportBoundary = "contract-import-boundary"
 	checkShimAlias      = "contract-shim-alias"
+	checkDataScopeRoute = "datascope-route-unguarded"
 )
 
 // Package paths, relative to the module. Spelled once so a module rename
@@ -47,6 +48,7 @@ func runChecks(s *snapshot, opt options) ([]Finding, error) {
 	out = append(out, checkMenuIDCollisions(s)...)
 	out = append(out, checkContractImportBoundary(s)...)
 	out = append(out, checkContractShimAlias(s)...)
+	out = append(out, checkDataScopeRoutes(s)...)
 
 	if opt.UIDir != "" {
 		fs, err := checkMenuNames(s, opt.UIDir)

tools/checksilent/datascope.go

@@ -0,0 +1,390 @@
+package main
+
+import (
+	"go/ast"
+	"strings"
+)
+
+// ---------------------------------------------------------------------------
+// check 8: a handler that reads the data permission, on a route that never
+// installs the middleware which puts one there
+// ---------------------------------------------------------------------------
+
+// permissionGetter is the function a handler calls to obtain the caller's data
+// scope, and permissionMiddleware is the middleware that puts one in the
+// context. Matched by name rather than by resolved symbol: the tool parses
+// without type checking, and both names are distinctive enough that a
+// same-named function from somewhere else would still be worth a look.
+const (
+	permissionGetter     = "GetPermissionFromContext"
+	permissionMiddleware = "PermissionAction"
+)
+
+// actionsPkgSuffix identifies the package the two names above live in - this
+// repository's common/actions shim and core's sdk/contract/actions both end
+// this way, and a module rename changes neither.
+const actionsPkgSuffix = "/actions"
+
+// handlerKey identifies one handler method uniquely across packages, so that
+// two types named SysUser in different packages are not confused.
+type handlerKey struct {
+	Pkg  string
+	Type string
+	Func string
+}
+
+// checkDataScopeRoutes reports a route whose handler asks for the caller's data
+// permission while the group it is registered on never installs the middleware
+// that supplies one.
+//
+// GetPermissionFromContext cannot fail. When nothing put a *DataPermission in
+// the context it hands back a zero value, whose DataScope is the empty string -
+// and the empty string is not one of the five scopes Permission recognises, so
+// it takes the default branch. That branch fails closed: the query is given
+// `1 = 0` and matches nothing.
+//
+// The result is an endpoint that answers "not found" or "no permission" for
+// rows that plainly exist, and only on deployments that set enabledp: true -
+// with data permissions off, Permission returns the query untouched and the
+// missing middleware costs nothing. That is the shape this check exists for: a
+// default configuration where the mistake is invisible, and a test suite that
+// runs on it.
+//
+// It happened. /api/v1/getinfo read the permission on a group carrying only the
+// JWT middleware, so every login on a deployment with data permissions enabled
+// ended in a 401 from the endpoint the browser calls immediately after signing
+// in - and went back to the login page.
+//
+// Either half is a fix, and which one depends on the route. A handler that
+// reads somebody else's rows wants the middleware. A handler reading the
+// caller's own row - where the id comes from the token - wants no scope at all,
+// because a scope has nothing left to restrict there and DataScopeSelf, which
+// matches on create_by, would reject every user who did not create their own
+// account. The check reports the mismatch and leaves the choice.
+func checkDataScopeRoutes(s *snapshot) []Finding {
+	handlers := permissionReadingHandlers(s)
+	if len(handlers) == 0 {
+		return nil
+	}
+
+	var out []Finding
+	for _, sf := range s.Files {
+		if sf.isTest() {
+			continue
+		}
+		for _, decl := range sf.Syntax.Decls {
+			fn, ok := decl.(*ast.FuncDecl)
+			if !ok || fn.Body == nil {
+				continue
+			}
+			out = append(out, s.routeFindings(sf, fn, handlers)...)
+		}
+	}
+	return out
+}
+
+// permissionReadingHandlers collects every method whose body calls the getter.
+//
+// Test files are included deliberately: a handler is a handler wherever it is
+// declared, and skipping them would let a route registered from a test fixture
+// go unchecked while the fixture is exactly where a new one gets written first.
+func permissionReadingHandlers(s *snapshot) map[handlerKey]bool {
+	out := map[handlerKey]bool{}
+	for _, sf := range s.Files {
+		for _, decl := range sf.Syntax.Decls {
+			fn, ok := decl.(*ast.FuncDecl)
+			if !ok || fn.Body == nil || fn.Recv == nil || len(fn.Recv.List) == 0 {
+				continue
+			}
+			recv := receiverTypeName(fn.Recv.List[0].Type)
+			if recv == "" {
+				continue
+			}
+			if callsPackageFunc(sf, fn.Body, permissionGetter) {
+				out[handlerKey{Pkg: sf.Pkg, Type: recv, Func: fn.Name.Name}] = true
+			}
+		}
+	}
+	return out
+}
+
+// routeFindings walks one function looking for group definitions and the routes
+// registered on them.
+func (s *snapshot) routeFindings(sf *sourceFile, fn *ast.FuncDecl, handlers map[handlerKey]bool) []Finding {
+	// Local variable bindings for the whole function. The first pass below
+	// fills these and the second reads them, so a registration sees every
+	// binding in the function rather than only the ones written above it -
+	// deliberately, because a `.Use` can be written below a route and still be
+	// part of the chain. The cost is that a name reused for two different
+	// things in one function resolves to whichever assignment came last.
+	apiVars := map[string]handlerKey{} // var -> the type it holds
+	guarded := map[string]bool{}       // group var -> middleware installed
+	known := map[string]bool{}         // group var -> is a router group at all
+	prefix := map[string]string{}      // group var -> the path it was declared with
+
+	var out []Finding
+	ast.Inspect(fn.Body, func(n ast.Node) bool {
+		switch stmt := n.(type) {
+		case *ast.AssignStmt:
+			for i, lhs := range stmt.Lhs {
+				id, ok := lhs.(*ast.Ident)
+				if !ok || i >= len(stmt.Rhs) {
+					continue
+				}
+				rhs := stmt.Rhs[i]
+				if key, ok := apiTypeOf(sf, rhs); ok {
+					apiVars[id.Name] = key
+					continue
+				}
+				if parent, isGroup := groupSource(rhs); isGroup {
+					known[id.Name] = true
+					prefix[id.Name] = prefix[parent] + groupPath(rhs)
+					// A subgroup inherits whatever its parent already had:
+					// gin copies the parent's handler chain into the child.
+					guarded[id.Name] = guarded[parent] || containsCallNamed(rhs, permissionMiddleware)
+				}
+			}
+		case *ast.ExprStmt:
+			// A separate `g.Use(...)` after the group was defined.
+			call, ok := stmt.X.(*ast.CallExpr)
+			if !ok {
+				return true
+			}
+			if target, ok := receiverIdentOf(call, "Use"); ok && known[target] {
+				if containsCallNamed(call, permissionMiddleware) {
+					guarded[target] = true
+				}
+			}
+		}
+		return true
+	})
+
+	// Second pass for the registrations, so that a `.Use` written below a route
+	// still counts - the middleware chain is assembled before any request is
+	// served, not in source order.
+	ast.Inspect(fn.Body, func(n ast.Node) bool {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		gvar, method, ok := routeRegistration(call)
+		if !ok || !known[gvar] || guarded[gvar] {
+			return true
+		}
+		route, handlerVar, handlerName, ok := routeArgs(call)
+		if !ok {
+			return true
+		}
+		key, ok := apiVars[handlerVar]
+		if !ok {
+			return true
+		}
+		key.Func = handlerName
+		if !handlers[key] {
+			return true
+		}
+		out = append(out, s.finding(Error, checkDataScopeRoute, sf, call,
+			"%s %q is handled by %s.%s, which reads the caller's data permission,\n"+
+				"    but the group it is registered on never installs %s.\n"+
+				"    GetPermissionFromContext then returns the zero value, whose empty DataScope is not a\n"+
+				"    recognised scope, so Permission fails closed and the query matches nothing - on any\n"+
+				"    deployment with enabledp: true. With data permissions off the route works, which is\n"+
+				"    why this does not show up in the default configuration or in CI.\n"+
+				"    Add %s() to the group, or stop scoping a query that is already limited to the caller.",
+			method, prefix[gvar]+route, key.Type, handlerName, permissionMiddleware, permissionMiddleware))
+		return true
+	})
+	return out
+}
+
+// receiverTypeName returns the bare type name of a method receiver, for both
+// `(e SysUser)` and `(e *SysUser)`.
+func receiverTypeName(expr ast.Expr) string {
+	if star, ok := expr.(*ast.StarExpr); ok {
+		expr = star.X
+	}
+	if id, ok := expr.(*ast.Ident); ok {
+		return id.Name
+	}
+	return ""
+}
+
+// callsPackageFunc reports whether body calls name on a package whose import
+// path ends in actionsPkgSuffix.
+func callsPackageFunc(sf *sourceFile, body ast.Node, name string) bool {
+	found := false
+	ast.Inspect(body, func(n ast.Node) bool {
+		if found {
+			return false
+		}
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		sel, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok || sel.Sel.Name != name {
+			return true
+		}
+		pkg, ok := sel.X.(*ast.Ident)
+		if !ok {
+			return true
+		}
+		if path, ok := sf.imports[pkg.Name]; ok && strings.HasSuffix(path, actionsPkgSuffix) {
+			found = true
+			return false
+		}
+		return true
+	})
+	return found
+}
+
+// apiTypeOf recognises `apis.SysUser{}` and returns the package path and type.
+func apiTypeOf(sf *sourceFile, expr ast.Expr) (handlerKey, bool) {
+	lit, ok := expr.(*ast.CompositeLit)
+	if !ok {
+		return handlerKey{}, false
+	}
+	sel, ok := lit.Type.(*ast.SelectorExpr)
+	if !ok {
+		return handlerKey{}, false
+	}
+	pkg, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return handlerKey{}, false
+	}
+	path, ok := sf.imports[pkg.Name]
+	if !ok {
+		return handlerKey{}, false
+	}
+	return handlerKey{Pkg: path, Type: sel.Sel.Name}, true
+}
+
+// groupSource reports whether expr builds a router group, and names the
+// variable it was built from when there is one.
+func groupSource(expr ast.Expr) (string, bool) {
+	parent := ""
+	isGroup := false
+	ast.Inspect(expr, func(n ast.Node) bool {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		sel, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok || sel.Sel.Name != "Group" {
+			return true
+		}
+		isGroup = true
+		if id, ok := sel.X.(*ast.Ident); ok {
+			parent = id.Name
+		}
+		return true
+	})
+	return parent, isGroup
+}
+
+// groupPath returns the literal path a group was declared with, or "" when it
+// is not a literal - a computed prefix is left out of the message rather than
+// printed as something it is not.
+func groupPath(expr ast.Expr) string {
+	out := ""
+	ast.Inspect(expr, func(n ast.Node) bool {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		sel, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok || sel.Sel.Name != "Group" || len(call.Args) == 0 {
+			return true
+		}
+		if lit, ok := call.Args[0].(*ast.BasicLit); ok {
+			out = strings.Trim(lit.Value, `"`)
+		}
+		return true
+	})
+	return out
+}
+
+// containsCallNamed reports whether expr contains a call to a function with
+// this name, at any depth of a method chain or argument list.
+func containsCallNamed(expr ast.Node, name string) bool {
+	found := false
+	ast.Inspect(expr, func(n ast.Node) bool {
+		if found {
+			return false
+		}
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+		switch fun := call.Fun.(type) {
+		case *ast.SelectorExpr:
+			if fun.Sel.Name == name {
+				found = true
+				return false
+			}
+		case *ast.Ident:
+			if fun.Name == name {
+				found = true
+				return false
+			}
+		}
+		return true
+	})
+	return found
+}
+
+// receiverIdentOf returns the variable a `x.method(...)` call was made on.
+func receiverIdentOf(call *ast.CallExpr, method string) (string, bool) {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok || sel.Sel.Name != method {
+		return "", false
+	}
+	id, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return "", false
+	}
+	return id.Name, true
+}
+
+// httpMethods are the registration calls this check understands. Any and Match
+// are absent on purpose: they take the method as data, and a check that half
+// understands a registration is worse than one that says nothing about it.
+var httpMethods = map[string]bool{
+	"GET": true, "POST": true, "PUT": true, "DELETE": true, "PATCH": true, "HEAD": true, "OPTIONS": true,
+}
+
+// routeRegistration recognises `g.GET(...)` and names the group and method.
+func routeRegistration(call *ast.CallExpr) (string, string, bool) {
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok || !httpMethods[sel.Sel.Name] {
+		return "", "", false
+	}
+	id, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return "", "", false
+	}
+	return id.Name, sel.Sel.Name, true
+}
+
+// routeArgs pulls the path and the `api.Handler` argument out of a
+// registration, ignoring any middleware written between them.
+func routeArgs(call *ast.CallExpr) (route, handlerVar, handlerName string, ok bool) {
+	if len(call.Args) < 2 {
+		return "", "", "", false
+	}
+	lit, isLit := call.Args[0].(*ast.BasicLit)
+	if !isLit {
+		return "", "", "", false
+	}
+	route = strings.Trim(lit.Value, `"`)
+	// The handler is the last argument; anything before it is middleware.
+	sel, isSel := call.Args[len(call.Args)-1].(*ast.SelectorExpr)
+	if !isSel {
+		return "", "", "", false
+	}
+	id, isIdent := sel.X.(*ast.Ident)
+	if !isIdent {
+		return "", "", "", false
+	}
+	return route, id.Name, sel.Sel.Name, true
+}

tools/checksilent/datascope_test.go

@@ -0,0 +1,163 @@
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+// apisFile is a handler package with two methods: one that reads the caller's
+// data permission and one that does not.
+const apisFile = `package apis
+
+import (
+	"github.com/gin-gonic/gin"
+	"go-admin/common/actions"
+)
+
+type SysUser struct{}
+
+func (e SysUser) Scoped(c *gin.Context) {
+	p := actions.GetPermissionFromContext(c)
+	_ = p
+}
+
+func (e SysUser) Unscoped(c *gin.Context) {}
+`
+
+func routerFile(uses string) string {
+	return `package router
+
+import (
+	"github.com/gin-gonic/gin"
+	"go-admin/app/admin/apis"
+	"go-admin/common/actions"
+)
+
+var _ = actions.PermissionAction
+
+func register(v1 *gin.RouterGroup) {
+	api := apis.SysUser{}
+	r := v1.Group("/sys-user")` + uses + `
+	{
+		r.GET("/:id", api.Scoped)
+	}
+}
+`
+}
+
+// The mistake itself: a handler that reads the permission, on a group that
+// never installs the middleware which puts one there.
+func TestDataScopeRouteWithoutTheMiddlewareIsReported(t *testing.T) {
+	root := fixture(t, map[string]string{
+		"app/admin/apis/sys_user.go":   apisFile,
+		"app/admin/router/sys_user.go": routerFile(`.Use(gin.Logger())`),
+	})
+	f := requireOne(t, check(t, root, options{}), checkDataScopeRoute)
+	for _, want := range []string{`GET "/sys-user/:id"`, "SysUser.Scoped", "PermissionAction"} {
+		if !strings.Contains(f.Message, want) {
+			t.Errorf("message does not mention %q:\n%s", want, f.Message)
+		}
+	}
+}
+
+// The middleware installed in the chain is the fix, and must silence it.
+func TestDataScopeRouteWithTheMiddlewareIsQuiet(t *testing.T) {
+	root := fixture(t, map[string]string{
+		"app/admin/apis/sys_user.go":   apisFile,
+		"app/admin/router/sys_user.go": routerFile(`.Use(gin.Logger()).Use(actions.PermissionAction())`),
+	})
+	if got := only(t, check(t, root, options{}), checkDataScopeRoute); len(got) != 0 {
+		t.Errorf("reported %d findings for a guarded group:\n%v", len(got), got)
+	}
+}
+
+// The other fix - the handler stops reading the permission - must silence it
+// too. Reporting a route whose handler needs no scope would push people to
+// install middleware they do not want, which is how /getinfo would have been
+// "fixed" into rejecting every user who did not create their own account.
+func TestARouteWhoseHandlerReadsNoPermissionIsQuiet(t *testing.T) {
+	root := fixture(t, map[string]string{
+		"app/admin/apis/sys_user.go": apisFile,
+		"app/admin/router/sys_user.go": `package router
+
+import (
+	"github.com/gin-gonic/gin"
+	"go-admin/app/admin/apis"
+)
+
+func register(v1 *gin.RouterGroup) {
+	api := apis.SysUser{}
+	r := v1.Group("")
+	{
+		r.GET("/getinfo", api.Unscoped)
+	}
+}
+`,
+	})
+	if got := only(t, check(t, root, options{}), checkDataScopeRoute); len(got) != 0 {
+		t.Errorf("reported %d findings for a handler that reads no permission:\n%v", len(got), got)
+	}
+}
+
+// gin copies the parent's handler chain into a subgroup, so a group carved out
+// of a guarded one is guarded. Reporting it would be a false positive, and a
+// check that cries wolf is one people switch off.
+func TestASubgroupInheritsTheMiddleware(t *testing.T) {
+	root := fixture(t, map[string]string{
+		"app/admin/apis/sys_user.go": apisFile,
+		"app/admin/router/sys_user.go": `package router
+
+import (
+	"github.com/gin-gonic/gin"
+	"go-admin/app/admin/apis"
+	"go-admin/common/actions"
+)
+
+func register(v1 *gin.RouterGroup) {
+	api := apis.SysUser{}
+	parent := v1.Group("/sys").Use(actions.PermissionAction())
+	child := parent.Group("/user")
+	{
+		child.GET("/:id", api.Scoped)
+	}
+}
+`,
+	})
+	if got := only(t, check(t, root, options{}), checkDataScopeRoute); len(got) != 0 {
+		t.Errorf("reported %d findings for a subgroup of a guarded group:\n%v", len(got), got)
+	}
+}
+
+// Two packages can both declare a SysUser. Only the one whose method reads the
+// permission may be reported, or the check becomes a name search.
+func TestAHandlerIsMatchedByPackageNotJustName(t *testing.T) {
+	root := fixture(t, map[string]string{
+		"app/admin/apis/sys_user.go": apisFile,
+		"app/other/apis/sys_user.go": `package apis
+
+import "github.com/gin-gonic/gin"
+
+type SysUser struct{}
+
+func (e SysUser) Scoped(c *gin.Context) {}
+`,
+		"app/other/router/sys_user.go": `package router
+
+import (
+	"github.com/gin-gonic/gin"
+	"go-admin/app/other/apis"
+)
+
+func register(v1 *gin.RouterGroup) {
+	api := apis.SysUser{}
+	r := v1.Group("/other")
+	{
+		r.GET("/:id", api.Scoped)
+	}
+}
+`,
+	})
+	if got := only(t, check(t, root, options{}), checkDataScopeRoute); len(got) != 0 {
+		t.Errorf("reported %d findings for a same-named handler in another package:\n%v", len(got), got)
+	}
+}