check_rbac.go

@@ -16,25 +16,10 @@ import (
 // finding.
 const coreSDKModulePath = "git.dev.alexdunmow.com/block/core"
 
-// definitionsOnlyModulePaths are Go modules that carry proto service/rpc
+// isDefinitionsOnlySDKTarget reports whether the backend target is the shared
-// definitions but serve NONE of them — RBAC is enforced in the repos that
+// proto SDK (core): it carries proto but serves nothing, so it has neither an
-// actually serve the procedures (cms for blockninja.v1, orchestrator for
+// RBAC interceptor nor a designated serving package prefix. Such a target is
-// orchestrator.v1). Scanning them in isolation would flag every procedure as
+// out of scope for check 2 — see coreSDKModulePath.
-// "missing from MethodRoles", a not-applicable result rather than a finding.
-//
-//   - core: the shared proto SDK.
-//   - cli:  the ninja developer CLI (WO-WZ-023). It vendors
-//     orchestrator/v1/plugin_registry.proto to generate a CLIENT for the
-//     registry; it has no server and no RBAC interceptor.
-var definitionsOnlyModulePaths = map[string]bool{
-	coreSDKModulePath:                  true,
-	"git.dev.alexdunmow.com/block/cli": true,
-}
-
-// isDefinitionsOnlySDKTarget reports whether the backend target 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
-// definitionsOnlyModulePaths.
 func isDefinitionsOnlySDKTarget(target backendScanTarget) bool {
 	if len(target.allowedPackagePrefixes) > 0 {
 		return false // a designated serving backend (cms/orchestrator)
@@ -46,7 +31,7 @@ func isDefinitionsOnlySDKTarget(target backendScanTarget) bool {
 	if err != nil {
 		return false
 	}
-	return definitionsOnlyModulePaths[modPath]
+	return modPath == coreSDKModulePath
 }
 
 func init() {
@@ -58,12 +43,6 @@ func init() {
 			protoMethods := make([]string, 0)
 			rbacMethods := make(map[string]string)
 			protoMethodSet := make(map[string]bool)
-			// protoMethodSetAll holds every declared proto procedure (unfiltered
-			// by served-surface). Stale detection compares RBAC entries against
-			// this full set so an RBAC role for a real-but-unmounted proto method
-			// (e.g. orchestrator.v1.DNSService, declared + roled but not served)
-			// is not mistaken for a dangling entry.
-			protoMethodSetAll := make(map[string]bool)
 			requiresRPCDiscovery := false
 			var rbacSummaries []rbacTargetSummary
 			var definitionsOnlySDKLabels []string
@@ -91,20 +70,6 @@ func init() {
 					if err != nil {
 						rep.Fatal("failed to parse proto files for %s: %v", target.displayOrRoot(), err)
 					}
-					// Record the FULL declared surface for stale detection, then
-					// scope the missing-RBAC check to the procedures this target
-					// actually SERVES. A serving backend may carry proto for
-					// services it never mounts (for frontend codegen); those are
-					// not real RBAC gaps. See extractServedServiceKeys.
-					summary.protoMethodsAll = append(summary.protoMethodsAll, targetProtoMethods...)
-					for _, pm := range targetProtoMethods {
-						protoMethodSetAll[pm] = true
-					}
-					servedKeys, err := extractServedServiceKeys(target.root)
-					if err != nil {
-						rep.Fatal("failed to scan served handlers for %s: %v", target.displayOrRoot(), err)
-					}
-					targetProtoMethods = filterProceduresToServed(targetProtoMethods, servedKeys)
 					summary.protoMethods = append(summary.protoMethods, targetProtoMethods...)
 					for _, pm := range targetProtoMethods {
 						if protoMethodSet[pm] {
@@ -134,22 +99,6 @@ func init() {
 				if err != nil {
 					rep.Fatal("failed to parse proto files for %s: %v", target.displayOrRoot(), err)
 				}
-				// Same served-surface scoping as the interceptor branch above:
-				// keep only procedures whose service this target actually mounts.
-				// This is the case the orchestrator hits — its RBAC interceptor
-				// lives under backend/ (so hasCoreRBAC is false at this root),
-				// and it carries the full blockninja.v1 + helpdesk.v1 proto while
-				// serving only orchestrator.v1. The full surface is retained for
-				// stale detection.
-				summary.protoMethodsAll = append(summary.protoMethodsAll, targetProtoMethods...)
-				for _, pm := range targetProtoMethods {
-					protoMethodSetAll[pm] = true
-				}
-				servedKeys, err := extractServedServiceKeys(target.root)
-				if err != nil {
-					rep.Fatal("failed to scan served handlers for %s: %v", target.displayOrRoot(), err)
-				}
-				targetProtoMethods = filterProceduresToServed(targetProtoMethods, servedKeys)
 				summary.protoMethods = append(summary.protoMethods, targetProtoMethods...)
 				for _, pm := range targetProtoMethods {
 					if protoMethodSet[pm] {
@@ -179,12 +128,8 @@ func init() {
 				if err != nil {
 					rep.Fatal("failed to parse plugin proto files for %s: %v", target.display, err)
 				}
-				// Plugins serve every procedure they declare — no served-surface
-				// filtering. The full and served sets are identical here.
 				summary.protoMethods = append(summary.protoMethods, pluginProtoMethods...)
-				summary.protoMethodsAll = append(summary.protoMethodsAll, pluginProtoMethods...)
 				for _, pm := range pluginProtoMethods {
-					protoMethodSetAll[pm] = true
 					if protoMethodSet[pm] {
 						continue
 					}
@@ -223,13 +168,9 @@ func init() {
 					}
 				}
 
-				// Stale = an RBAC entry that matches no DECLARED proto method at
-				// all (compare against the full surface, not the served subset —
-				// a role for a real-but-unmounted proto method is intentional, not
-				// dangling).
 				var stale []string
 				for m := range rbacMethods {
-					if !protoMethodSetAll[m] {
+					if !protoMethodSet[m] {
 						if isExcludedServiceMethod(m) {
 							continue
 						}
@@ -251,9 +192,7 @@ func init() {
 						if isExcludedServiceMethod(method) {
 							continue
 						}
-						// Compare against the full declared surface (see the
+						if !containsString(summary.protoMethods, method) {
-						// global stale note above), not the served subset.
-						if !containsString(summary.protoMethodsAll, method) {
 							summary.staleCount++
 						}
 					}

colors.go

@@ -187,21 +187,6 @@ func init() {
 }
 
 func isColorAllowed(relPath string) bool {
-	// The color allowlist keys are source-relative (e.g.
-	// "components/settings/theme-card.tsx"), but relPath is computed relative to
-	// whatever scan root the caller used. That root varies by repo: the CMS
-	// frontend scan root is "web/src" (relPath = "components/..."), while the
-	// orchestrator's is "frontend" (relPath = "src/components/..."). Trim any of
-	// these leading source prefixes so one allowlist entry matches regardless of
-	// which repo is the primary scan target. Mirrors isAllowedPlaceholderFile's
-	// web/src trimming.
-	normalized := filepath.ToSlash(relPath)
-	normalized = strings.TrimPrefix(normalized, "web/src/")
-	normalized = strings.TrimPrefix(normalized, "frontend/src/")
-	normalized = strings.TrimPrefix(normalized, "src/")
-	if colorAllowedFiles[normalized] {
-		return true
-	}
 	if colorAllowedFiles[relPath] {
 		return true
 	}

comingsoon.go

@@ -40,15 +40,6 @@ var placeholderCopyPatterns = []*regexp.Regexp{
 var rePlaceholderComment = regexp.MustCompile(`(?i)\bplaceholder\b`)
 var rePlaceholderStringLiteral = regexp.MustCompile("`[^`]*\\bplaceholder\\b[^`]*`|\"[^\"]*\\bplaceholder\\b[^\"]*\"|'[^']*\\bplaceholder\\b[^']*'")
 
-// isGeneratedProtoOutput reports whether path lives under a `gen/` directory
-// segment — the buf codegen output tree (e.g. `.../lib/api/gen/blockninja/v1/`).
-// Scoped to the placeholder/"ship the feature now" check only: generated proto
-// code is never hand-authored, so its symbols (COMING_SOON enum values, etc.)
-// must not be flagged as placeholder debt.
-func isGeneratedProtoOutput(path string) bool {
-	return strings.Contains(filepath.ToSlash(path), "/gen/")
-}
-
 func checkComingSoon(webSrcDir string) []comingSoonViolation {
 	var violations []comingSoonViolation
 
@@ -70,14 +61,6 @@ func checkComingSoon(webSrcDir string) []comingSoonViolation {
 		if strings.HasSuffix(path, ".test.ts") || strings.HasSuffix(path, ".test.tsx") {
 			return nil
 		}
-		// Skip buf codegen output (any `gen/` directory segment, e.g.
-		// lib/api/gen/blockninja/v1/*_pb.d.ts). These files are regenerated by
-		// buf and must never be hand-edited; their enum values like
-		// SYSTEM_PAGE_TYPE_COMING_SOON / SITE_MODE_COMING_SOON are real proto
-		// symbols, not authored placeholder debt.
-		if isGeneratedProtoOutput(path) {
-			return nil
-		}
 
 		relPath, _ := filepath.Rel(webSrcDir, path)
 		if relPath == "" {
@@ -158,8 +141,7 @@ func checkPlaceholderLanguage(roots []todoScanRoot) []comingSoonViolation {
 			slashPath := filepath.ToSlash(path)
 			if strings.Contains(slashPath, "/generated/") ||
 				strings.HasSuffix(slashPath, ".pb.go") ||
-				strings.HasSuffix(slashPath, ".connect.go") ||
+				strings.HasSuffix(slashPath, ".connect.go") {
-				isGeneratedProtoOutput(path) {
 				return nil
 			}
 			if strings.Contains(filepath.ToSlash(path), "cmd/check-safety/") {

frontend.go

@@ -52,11 +52,8 @@ var allowedFrontendFiles = map[string]bool{
 // Plugin web files that are allowed to call plugin-owned REST handlers because
 // there is no ConnectRPC surface for them yet. Keep this list narrow.
 var allowedPluginRESTFiles = map[string]bool{
-	// Suffix-matched (see pluginRESTFileAllowed), so these cover calcomblock both
+	"backend/internal/plugins/calcomblock/web/settings.tsx": true, // Plugin settings/test endpoints are mounted via HTTPHandler; no generated hooks exist yet
-	// as the standalone plugin repo (plugins/calcomblock/web/...) and the legacy
+	"backend/internal/plugins/calcomblock/web/editor.tsx":   true, // Block editor reads plugin /settings + /event-types via HTTPHandler routes
-	// bundled path (backend/internal/plugins/calcomblock/web/...).
-	"plugins/calcomblock/web/settings.tsx": true, // Plugin settings/test endpoints are mounted via HTTPHandler; no generated hooks exist yet
-	"plugins/calcomblock/web/editor.tsx":   true, // Block editor reads plugin /settings + /event-types via HTTPHandler routes
 }
 
 // Specific fetch paths that are non-proto REST endpoints (no ConnectRPC equivalent)
@@ -70,7 +67,6 @@ var knownNonProtoFetches = map[string]bool{
 	"/api/ai/chat/stream":  true, // AI chat SSE streaming (EventSource/fetch — ConnectRPC doesn't support SSE)
 	"/api/support/":        true, // CMS helpdesk widget: multipart attachment upload to the Chi proxy — connect-query can't send FormData
 	"/api/helpdesk/upload": true, // Orchestrator helpdesk: multipart attachment upload — same FormData limitation
-	"/.well-known/skills/": true, // Public well-known skill discovery index (instance-derived skill name) — no proto service
 }
 
 var (

frontend_extra.go

@@ -213,17 +213,6 @@ type lockfileViolation struct {
 	file string
 }
 
-// allowedLockfiles are non-pnpm lockfiles intentionally tolerated, keyed by
-// repo-relative slash path. These live in gitignored tool working dirs (never
-// committed) and are regenerated by an external toolchain we don't control, so
-// they are not part of the repo's dependency surface.
-var allowedLockfiles = map[string]bool{
-	// cms/styles is a gitignored tailwind-service working dir; its
-	// package-lock.json is regenerated by the tailwind tooling and never
-	// committed (see cms/.gitignore).
-	"styles/package-lock.json": true,
-}
-
 func checkLockfiles(repoRoot string) []lockfileViolation {
 	var violations []lockfileViolation
 
@@ -237,11 +226,7 @@ func checkLockfiles(repoRoot string) []lockfileViolation {
 		base := filepath.Base(path)
 		if base == "package-lock.json" || base == "yarn.lock" {
 			relPath, _ := filepath.Rel(repoRoot, path)
-			rel := filepath.ToSlash(relPath)
+			violations = append(violations, lockfileViolation{file: relPath})
-			if allowedLockfiles[rel] {
-				return nil
-			}
-			violations = append(violations, lockfileViolation{file: rel})
 		}
 		return nil
 	}); err != nil {

proto_rbac.go

@@ -711,118 +711,6 @@ func normalizeRoleName(raw string) string {
 	return strings.ToLower(raw)
 }
 
-// servedHandlerRefPattern matches a PACKAGE-QUALIFIED connect handler
-// constructor reference, e.g. `orchestratorv1connect.NewSettingsServiceHandler`.
-//
-// A connect-go service is served by a repo iff its generated
-// New{Service}Handler constructor is referenced (called) in that repo's
-// HAND-WRITTEN backend code — that reference is how the handler gets mounted on
-// the mux. The generated `*.connect.go` stubs DEFINE that constructor for every
-// service the repo carries proto for, so a repo that vendors the whole
-// blockninja.v1 surface only for frontend codegen (the orchestrator) still
-// SERVES just the subset it actually mounts.
-//
-// We keep the package selector (`…connect`) rather than only the short service
-// name because two proto packages can declare services that share a short name
-// (`blockninja.v1.SettingsService` vs `orchestrator.v1.SettingsService`); the
-// selector is what disambiguates which one is really mounted.
-var servedHandlerRefPattern = regexp.MustCompile(`([A-Za-z0-9_]+connect)\.New([A-Za-z0-9_]+)ServiceHandler\b`)
-
-// extractServedServiceKeys scans a backend target's HAND-WRITTEN Go source for
-// package-qualified connect handler mounts and returns the set of served
-// services, keyed as "<connectPkg>|<ServiceName>" (e.g.
-// "orchestratorv1connect|SettingsService").
-//
-// Generated `*.connect.go` stubs and `_test.go` files are skipped: the stubs
-// only DEFINE the constructors (unqualified `func New…ServiceHandler`) and would
-// otherwise mark every carried service as served; tests may reference handlers
-// for services that aren't mounted in the real server.
-func extractServedServiceKeys(root string) (map[string]bool, error) {
-	served := make(map[string]bool)
-	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if info.IsDir() {
-			switch info.Name() {
-			case ".git", ".worktrees", "vendor", "node_modules":
-				return filepath.SkipDir
-			}
-			return nil
-		}
-		if !strings.HasSuffix(path, ".go") {
-			return nil
-		}
-		if strings.HasSuffix(path, ".connect.go") || strings.HasSuffix(path, "_test.go") {
-			return nil
-		}
-		data, err := os.ReadFile(path)
-		if err != nil {
-			return nil // skip unreadable files
-		}
-		for _, m := range servedHandlerRefPattern.FindAllSubmatch(data, -1) {
-			connectPkg := string(m[1])
-			service := string(m[2]) + "Service"
-			served[connectPkg+"|"+service] = true
-		}
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-	return served, nil
-}
-
-// procedureServedKey maps a proto procedure ("/pkg.sub.ServiceName/Method") to
-// the served-service key produced by extractServedServiceKeys, deriving the
-// connect package name from the proto package exactly the way
-// protoc-gen-connect-go does: strip the dots and append "connect"
-// (blockninja.v1 -> blockninjav1connect, orchestrator.v1 ->
-// orchestratorv1connect). Returns false for procedures without a package.
-func procedureServedKey(procedure string) (string, bool) {
-	trimmed := strings.TrimPrefix(procedure, "/")
-	parts := strings.SplitN(trimmed, "/", 2)
-	if len(parts) != 2 || parts[0] == "" {
-		return "", false
-	}
-	serviceFull := parts[0] // e.g. orchestrator.v1.SettingsService
-	lastDot := strings.LastIndex(serviceFull, ".")
-	if lastDot <= 0 || lastDot == len(serviceFull)-1 {
-		return "", false
-	}
-	protoPkg := serviceFull[:lastDot]      // orchestrator.v1
-	serviceName := serviceFull[lastDot+1:] // SettingsService
-	connectPkg := strings.ReplaceAll(protoPkg, ".", "") + "connect"
-	return connectPkg + "|" + serviceName, true
-}
-
-// filterProceduresToServed keeps only procedures whose service is actually
-// mounted (served) by the target, per the served-service key set. This is what
-// scopes check 2 to a repo's real serving surface: a repo may CARRY proto for
-// procedures it never mounts (the orchestrator carries all of blockninja.v1 and
-// helpdesk.v1 for frontend codegen but serves only orchestrator.v1), and those
-// unmounted procedures are not real RBAC gaps.
-//
-// If the served set is empty — no recognised handler mounts, e.g. a target that
-// mounts via a pattern we don't detect — the procedures are returned UNCHANGED
-// so we never silently hide a genuine missing-RBAC finding.
-func filterProceduresToServed(procedures []string, served map[string]bool) []string {
-	if len(served) == 0 {
-		return procedures
-	}
-	out := make([]string, 0, len(procedures))
-	for _, p := range procedures {
-		key, ok := procedureServedKey(p)
-		if !ok {
-			continue
-		}
-		if served[key] {
-			out = append(out, p)
-		}
-	}
-	return out
-}
-
 func isExcludedServiceMethod(method string) bool {
 	parts := strings.Split(method, "/")
 	if len(parts) < 3 {

targets.go

@@ -28,19 +28,12 @@ type frontendScanTarget struct {
 }
 
 type rbacTargetSummary struct {
-	label string
+	label        string
-	// protoMethods is the SERVED procedure surface (filtered to services the
-	// target actually mounts) — it drives the missing-RBAC (permission_denied)
-	// finding.
 	protoMethods []string
-	// protoMethodsAll is the FULL declared procedure surface (unfiltered) — it
+	roleByMethod map[string]string
-	// drives stale-entry detection, so an RBAC role for a real-but-unmounted
+	missingCount int
-	// proto method is not reported as stale.
+	staleCount   int
-	protoMethodsAll []string
+	requiresRPCs bool
-	roleByMethod    map[string]string
-	missingCount    int
-	staleCount      int
-	requiresRPCs    bool
 }
 
 func (t backendScanTarget) displayOrRoot() string {
@@ -69,17 +62,6 @@ func resolveScanRoots(target string) (backendDir string, repoRoot string, err er
 		return filepath.Join(absTarget, "backend"), absTarget, nil
 	}
 
-	// Orchestrator layout — same standalone-app shape as the CMS above, but the
-	// frontend lives at frontend/src (not web/src) and proto/buf sit at the repo
-	// root. Without this clause the orchestrator root resolves as its own backend
-	// (backendDir == repoRoot): backend/go.mod is reported missing, and
-	// frontend/src is treated as a plugin frontend so the plugin-only rules fire
-	// on a first-class app.
-	if fileExists(filepath.Join(absTarget, "backend", "go.mod")) && dirExists(filepath.Join(absTarget, "frontend", "src")) &&
-		(fileExists(filepath.Join(absTarget, "buf.yaml")) || dirExists(filepath.Join(absTarget, "proto"))) {
-		return filepath.Join(absTarget, "backend"), absTarget, nil
-	}
-
 	if filepath.Base(absTarget) == "backend" && fileExists(filepath.Join(absTarget, "go.mod")) {
 		parent := filepath.Clean(filepath.Join(absTarget, ".."))
 		if fileExists(filepath.Join(parent, "buf.yaml")) || dirExists(filepath.Join(parent, "proto")) {