From 3c88b19ff638d1f7e7b3ef9bd1d822d19691448a Mon Sep 17 00:00:00 2001 From: Alex Dunmow Date: Thu, 2 Jul 2026 15:12:19 +0800 Subject: [PATCH] docs: make README all-inclusive (full 31-check table, all CLI flags) Co-Authored-By: Claude Opus 4.8 --- README.md | 111 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 90 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index fb1c5fb..a8876bd 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,11 @@ # check-safety -Static safety checker for the BlockNinja codebase. Walks a target tree, runs ~25 invariant checks across Go and frontend sources, and exits non-zero on violations. +Static safety checker for the BlockNinja codebase. Walks a target tree, runs 31 invariant +checks across Go and frontend sources, and exits non-zero on any violation. -Lives at `~/src/blockninja/check-safety/` as a standalone Go module, alongside `cms/`, `orchestrator/`, `core/`, etc. +Lives at `~/src/blockninja/check-safety/` as a standalone Go module, alongside `cms/`, +`orchestrator/`, `core/`, the plugins, and the sites. It is intentionally standalone — no +imports from sibling repos except two vendored packages (see [Vendored packages](#how-it-stays-in-sync-with-cms)). ## Run @@ -10,47 +13,113 @@ Lives at `~/src/blockninja/check-safety/` as a standalone Go module, alongside ` # Scan CMS (default target) make run -# Scan something else +# Scan a different repo make run TARGET=../orchestrator -# Direct +# Direct invocation — first positional arg is the target dir (default ".") go run . ../sites/bidbuddy +go run . ~/src/blockninja/cms ``` +If you run it against the top of the consolidated BlockNinja repo, it prints a hint to pass +`--plugin-dir ` (or run inside the plugin directory) so plugin-specific checks fire. + +### CLI flags + +| Flag | Meaning | +|------|---------| +| `` (positional) | Repo/subtree to scan. Defaults to `.`. If it contains `plugin.mod`, it is auto-registered as a plugin root so plugin checks run. | +| `--orchestrator` | Scan the orchestrator backend only (resolved from the hardcoded consolidated layout), regardless of the positional target. | +| `--plugin-dir [more…]` | Register one or more plugin roots (dirs with `plugin.mod`). Alias: `--plugin-dirs`. Consumes args until the next `--flag`. | +| `--plugin-pages [more…]` | Register frontend source dirs directly as plugin page targets (for the frontend checks). Consumes args until the next `--flag`. | + +The CLI contract (`check-safety [--flags]`) is stable — the CMS `Makefile` +`safety-check` / `install-safety-checker` targets shell into this directory and depend on it. + ## Install globally ```bash -make install # → $GOPATH/bin/check-safety +make install # go install . → $GOPATH/bin/check-safety check-safety ~/src/blockninja/cms ``` ## Test ```bash -make test # all tests (some shell out to npm/tsc/golangci-lint and may skip if absent) -make test-short # skip the long ones -make test-update # regenerate golden snapshots after intentional output changes +make test # go test -count=1 ./... (some tests shell out to npm/tsc/golangci-lint and skip if absent) +make test-short # go test -short — skip the slow ones +make test-update # regenerate the golden snapshots after intentional output changes +make tidy # go mod tidy +make build # go build -o check-safety . +make clean # remove the built binary ``` +`golden_test.go` is a characterisation snapshot test: it runs the whole binary against the +fixtures under `testdata/golden/` and diffs stdout + exit code. If you intentionally change a +check's output, run `make test-update` and commit the regenerated fixture. + ## What it checks -See the comment block at the top of `main.go` for the canonical list. Highlights: +Checks self-register via `init()` and run in `Seq` order. The full set (the `ID` is what the +tool prints in each check header): -- Secret env-var reads happen only inside `config.Load()` -- All RPC methods are registered in the RBAC interceptor -- Frontend uses generated ConnectRPC hooks, no hand-crafted clients -- No hardcoded colors, no `useState` for tab state, no npm/yarn lockfiles -- No raw SQL outside sqlc/Bob, no `err.Error()` leaked to HTTP clients -- Plugin presets validate against `theme.Theme` -- Standalone plugins stay on the published SDK boundary +| ID | Check | What it enforces | +|----|-------|------------------| +| 1 | Secret env var reads | Secret env vars (`JWT_SECRET`, `VAULT_*`, encryption keys, …) are read only inside `config.Load()` (`config/config.go`) or `_test.go` files. | +| 2 | RBAC registration | Every RPC method is registered in the RBAC interceptor. Internal / plugin-managed services (e.g. `ManagementService`, Symposium's `WikiService`) are exempted. | +| 2b | Plugin proto ownership | Plugins own their proto/RBAC definitions correctly (no poaching of core proto packages). | +| 2c | Plugin SDK boundaries | Standalone plugins import only the published `block/core` SDK boundary — verified via imports, `go.mod`, and version. | +| 2d | sqlc UUID overrides | sqlc UUID overrides in standalone plugins and `cmd` configs must use `github.com/google/uuid`. | +| 2e | `any` usage | Warns on `any` usage in Go and TypeScript (capped at 120 printed warnings). | +| 2f | Codegen freshness | `sqlc compile` and `buf generate` must succeed for every scanned root that defines them (generated code is up to date). | +| 3 | Go lint | Go code compiles and passes `go fix`, `golangci-lint --fix`, `go vet`, and the strict lint checks. | +| 3b | Orchestrator tests | Orchestrator backend tests must pass when checking the core BlockNinja repo. | +| 4 | Frontend lint | Frontend code is auto-formatted, lint-clean, and typecheck-clean. | +| 5 | ConnectRPC hooks | Frontend uses the generated ConnectRPC hooks — no hand-crafted API clients. | +| 6 | No hardcoded colors | No hardcoded colors in the frontend — use theme tokens. | +| 7 | Tab state in URL | No `useState` for tab state in route files — use URL `?tab=` params. | +| 8 | API subpath imports | Import `@block-ninja/api` from subpaths only, never the package root. | +| 9 | pnpm only | No npm/yarn lockfiles — pnpm is the only package manager. | +| 10 | Button automation | Interactive buttons carry the required automation attributes. | +| 10b | No eslint-disable | No `eslint-disable-next-line` comments (fix the lint, don't suppress it). | +| 10disc | Plugin discovery | Plugin frontend pages are discoverable by the loader. | +| 11 | No placeholder code | No placeholder / stub code — only shipped features. | +| 12 | No reinvented utils | No reinvented utilities — use the shared helpers. | +| 13 | RPC error handling | RPC query/mutation error handling is present and correct. | +| 14 | No raw SQL | No raw SQL outside sqlc / Bob. | +| 15 | No leaked errors | No `err.Error()` leaked to HTTP clients. | +| 16 | Directory structure | Services and handlers live in their correct directories. | +| 17 | No TODO markers | No TODO markers in production code. | +| 18 | Plugin segmentation | Plugin segmentation is respected (`safety-rules.yml`). | +| 19 | Auth precedence | Bearer token takes precedence over cookies in the auth middleware. | +| 20 | Tailwind v4 | Tailwind v4 configuration is correct (PostCSS plugin, CSS directives, `@config`). | +| 21 | Preset validation | Plugin `presets.json` type-safe-unmarshals against `theme.Theme`. | +| 22 | HTML sanitization | No hand-rolled HTML sanitization — use bluemonday. | +| 28 | Admin toolbar | Public page handlers inject the admin toolbar data. | + +> The canonical numbered list also lives in the comment block at the top of `main.go`; keep +> the two in sync when adding or renumbering a check. ## How it stays in sync with CMS -`internal/helpers/deferlog.go` and `internal/theme/*.go` are vendored copies from CMS (`cms/backend/internal/{helpers,theme}/`). Go's `internal/` rule blocks direct imports across modules, so they live here. When CMS changes the theme schema or `LogDeferredError`, re-copy them — that drift is the point of the preset-validation check. +`internal/helpers/deferlog.go` and `internal/theme/*.go` are **vendored copies** from CMS +(`cms/backend/internal/{helpers,theme}/`). Go's `internal/` rule blocks direct imports across +modules, so they are copied in rather than imported. When CMS changes the theme schema or +`LogDeferredError`, re-copy them — that drift is precisely what the preset-validation check +(21) surfaces. + +`blockNinjaRepoRoot()` and `orchestratorRepoRoot()` in `lint_pipeline.go` hardcode the +consolidated layout (`~/src/blockninja/{cms,orchestrator}`). Update them if the tree layout +changes. ## Adding a new check -1. Add a `check_.go` file with a `runCheck` func -2. Wire it into `main.go`'s check sequence -3. Add a goldens case in `golden_test.go` if the output is deterministic -4. Run `make test-update` to regenerate goldens +1. Add a `check_.go` file with a `runCheck` func and an `init()` that calls + `register(Check{Seq, ID, Title, Run})`. +2. Add the rule logic in a `.go` file, plus a `_test.go` for unit coverage. +3. Pick a `Seq` value that slots it into the desired run order. +4. Update the numbered comment block at the top of `main.go` and the table above. +5. If the output is deterministic, add a fixture under `testdata/golden//` and run + `make test-update` to regenerate the snapshot. + +