From 482e4f9d43e760f09d905b1da23089bdbad80b12 Mon Sep 17 00:00:00 2001 From: Alex Dunmow Date: Sat, 4 Jul 2026 09:28:19 +0800 Subject: [PATCH] fix(check2): skip RBAC check for the definitions-only core SDK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check 2 (RPC methods registered in RBAC interceptor) fired FAIL on core, flagging all 926 shared procedures as "NOT in MethodRoles". Core is the shared Go SDK: it carries the canonical proto (blockninja.v1, orchestrator.v1, helpdesk.v1) but serves nothing and has no interceptor, so there is no MethodRoles map to match against and every procedure looked missing. RBAC is genuinely enforced where these procedures are actually served, and check 2 already validates it there: cms covers blockninja.v1 (green) and orchestrator covers orchestrator.v1 (0 missing). The abi.v1 protos declare no services, and helpdesk.v1 is unmounted definitions. So scanning the SDK in isolation is a not-applicable result, not a finding. Detect the core SDK target (no serving package prefix, no interceptor.go, module path == core) and SKIP check 2 for it instead of folding its shared procedures into the missing-set. Serving backends (cms/orchestrator) and plugins are untouched — the guard short-circuits the moment a target has a package prefix or an interceptor. Co-Authored-By: Claude Opus 4.8 --- check_rbac.go | 44 ++++++++++++++++++++++++++++++- check_rbac_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 check_rbac_test.go diff --git a/check_rbac.go b/check_rbac.go index cc70fcc..c18023a 100644 --- a/check_rbac.go +++ b/check_rbac.go @@ -3,8 +3,37 @@ package main import ( "path/filepath" "sort" + "strings" ) +// coreSDKModulePath is the shared Go SDK that carries the canonical proto +// definitions (blockninja.v1, orchestrator.v1, helpdesk.v1) consumed by the +// serving backends. It has no connect server and no RBAC interceptor of its +// own — RBAC is enforced in the repos that actually serve these procedures +// (cms for blockninja.v1, orchestrator for orchestrator.v1), where check 2 +// runs against their interceptors. Scanning the SDK in isolation would flag +// every shared procedure as "missing"; that is a not-applicable result, not a +// finding. +const coreSDKModulePath = "git.dev.alexdunmow.com/block/core" + +// isDefinitionsOnlySDKTarget reports whether the backend target is the shared +// proto SDK (core): it carries proto but serves nothing, so it has neither an +// RBAC interceptor nor a designated serving package prefix. Such a target is +// out of scope for check 2 — see coreSDKModulePath. +func isDefinitionsOnlySDKTarget(target backendScanTarget) bool { + if len(target.allowedPackagePrefixes) > 0 { + return false // a designated serving backend (cms/orchestrator) + } + if fileExists(filepath.Join(target.root, "internal/rbac/interceptor.go")) { + return false // has a serving RBAC interceptor + } + modPath, err := readModulePath(target.root) + if err != nil { + return false + } + return modPath == coreSDKModulePath +} + func init() { register(Check{ Seq: 20, @@ -16,8 +45,17 @@ func init() { protoMethodSet := make(map[string]bool) requiresRPCDiscovery := false var rbacSummaries []rbacTargetSummary + var definitionsOnlySDKLabels []string for _, target := range ctx.backendTargets { + // The shared proto SDK (core) serves nothing — RBAC is enforced + // in the consuming repos, where this check runs. Don't fold its + // procedures into the missing-set; check 2 is not applicable. + if isDefinitionsOnlySDKTarget(target) { + definitionsOnlySDKLabels = append(definitionsOnlySDKLabels, target.displayOrRoot()) + continue + } + rbacFile := filepath.Join(target.root, "internal/rbac/interceptor.go") hasCoreRBAC := fileExists(rbacFile) summary := rbacTargetSummary{ @@ -114,7 +152,11 @@ func init() { if requiresRPCDiscovery { rep.Fatal("no RPC procedures found in proto sources or generated Connect files") } - rep.Skip("no proto-backed RPC procedures found in scanned target(s)") + if len(definitionsOnlySDKLabels) > 0 { + rep.Skip("%s — definitions-only proto SDK; RBAC enforced in the serving repos (cms/orchestrator), not here", strings.Join(definitionsOnlySDKLabels, ", ")) + } else { + rep.Skip("no proto-backed RPC procedures found in scanned target(s)") + } } else { var missing []string for _, m := range protoMethods { diff --git a/check_rbac_test.go b/check_rbac_test.go new file mode 100644 index 0000000..bccd6d2 --- /dev/null +++ b/check_rbac_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// writeModule writes a minimal go.mod declaring modulePath under dir. +func writeModule(t *testing.T, dir, modulePath string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module "+modulePath+"\n\ngo 1.23\n"), 0o644); err != nil { + t.Fatalf("write go.mod: %v", err) + } +} + +func TestIsDefinitionsOnlySDKTarget(t *testing.T) { + t.Run("core SDK: proto but no interceptor, no serving prefix -> skipped", func(t *testing.T) { + dir := t.TempDir() + writeModule(t, dir, coreSDKModulePath) + if !isDefinitionsOnlySDKTarget(backendScanTarget{root: dir}) { + t.Fatal("expected core SDK target to be definitions-only") + } + }) + + t.Run("serving backend with package prefix -> not skipped", func(t *testing.T) { + dir := t.TempDir() + writeModule(t, dir, coreSDKModulePath) // even the core module path must not skip if it declares a serving prefix + target := backendScanTarget{root: dir, allowedPackagePrefixes: []string{"blockninja."}} + if isDefinitionsOnlySDKTarget(target) { + t.Fatal("a target with a serving package prefix must never be treated as definitions-only") + } + }) + + t.Run("backend carrying an RBAC interceptor -> not skipped", func(t *testing.T) { + dir := t.TempDir() + writeModule(t, dir, coreSDKModulePath) + rbacDir := filepath.Join(dir, "internal", "rbac") + if err := os.MkdirAll(rbacDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(rbacDir, "interceptor.go"), []byte("package rbac\n"), 0o644); err != nil { + t.Fatalf("write interceptor: %v", err) + } + if isDefinitionsOnlySDKTarget(backendScanTarget{root: dir}) { + t.Fatal("a target with a serving RBAC interceptor must never be treated as definitions-only") + } + }) + + t.Run("some other SDK module -> not skipped", func(t *testing.T) { + dir := t.TempDir() + writeModule(t, dir, "git.dev.alexdunmow.com/block/cms") + if isDefinitionsOnlySDKTarget(backendScanTarget{root: dir}) { + t.Fatal("only the core SDK module is definitions-only; other modules must be checked") + } + }) + + t.Run("no go.mod -> not skipped", func(t *testing.T) { + dir := t.TempDir() + if isDefinitionsOnlySDKTarget(backendScanTarget{root: dir}) { + t.Fatal("a target without a resolvable module must not be treated as definitions-only") + } + }) +}