tiennm99/goclaw
1
0
Fork 0
mirror of https://github.com/tiennm99/goclaw.git synced 2026-09-10 04:19:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

docs(contrib): tenant-scope guard rules for admin writes

Browse Source 
Role checks are not tenant checks — a non-master tenant admin holds
RoleAdmin in their own tenant and passes role-only middleware by
design. CLAUDE.md gains a one-line directive pointing at CONTRIBUTING
for the full decision table + anti-patterns. CONTRIBUTING gains:

- Target-table decision table (global vs tenant-scoped) with the
  matching guard for each (requireMasterScope vs requireTenantAdmin).
- Shared predicate reference: store.IsMasterScope(ctx).
- Anti-pattern list for reviewers: writes to no-tenant_id tables
  without master-scope check, SQL tenant_id IS NULL arms on write
  paths, role-only admin gates, revoke/delete handlers that skip
  pre-fetch ownership verification.
This commit is contained in:
viettranx
2026-04-12 10:48:31 +07:00
parent 714600f2a9
commit 7ae9721c2b
2 changed files with 19 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CLAUDE.md
+1
View File
@@ -197,6 +197,7 @@ Go conventions to follow:
- **SQL safety:** When implementing or modifying SQL store code (`store/pg/*.go`), always verify: (1) All user inputs use parameterized queries (`$1, $2, ...`), never string concatenation — prevents SQL injection. (2) Queries are optimized — no N+1 queries, no unnecessary full table scans. (3) WHERE clauses, JOINs, and ORDER BY columns use existing indices — check migration files for available indexes
- **DB query reuse:** Before adding a new DB query for key entities (teams, agents, sessions, users), check if the same data is already fetched earlier in the current flow/pipeline. Prefer passing resolved data through context, event payloads, or function params rather than re-querying. Duplicate queries waste DB resources and add latency
- **Solution design:** When designing a fix or feature, identify the root cause first — don't just patch symptoms. Think through production scenarios (high concurrency, multi-tenant isolation, failure cascades, long-running sessions) to ensure the solution holds up. Prefer explicit configuration over runtime heuristics. Prefer the simplest solution that addresses the root cause directly
- **Tenant-scope guards on admin writes:** `RoleAdmin` is not a tenant check. Writes to **global** tables (no `tenant_id` column — e.g. `builtin_tools`, disk config, package mgmt) must gate with `http.requireMasterScope` / WS `requireMasterScope(requireOwner(...))`. Writes to **tenant-scoped** tables must gate with `http.requireTenantAdmin` + SQL `WHERE tenant_id = $N`. Shared predicate: `store.IsMasterScope(ctx)`. See `CONTRIBUTING.md` → "Tenant-scope guards" for the full decision table and anti-patterns.
## Mobile UI/UX Rules
CONTRIBUTING.md
+18 -1
View File
@@ -50,11 +50,28 @@ Based on our automated review checklist:
- **Correctness**: No logic errors, nil dereference, race conditions
- **Security**: Parameterized SQL, no hardcoded secrets, input validation
- **Breaking changes**: API contracts, DB migrations, config format
- **Tenant isolation**: All queries scoped by `tenant_id`
- **Tenant isolation**: All queries scoped by `tenant_id`. **Admin writes require the correct scope guard** — see section below
- **i18n**: User-facing strings in all 3 locales (en/vi/zh)
- **SQLite parity**: Changes compile with `-tags sqliteonly`
- **Mobile UI**: `h-dvh` not `h-screen`, 16px input fonts, safe areas
### Tenant-Scope Guards
`RoleAdmin` checks role, not tenant. A non-master tenant admin holds `RoleAdmin` in their own tenant and passes role-only middleware. Pick the guard by the **target table**:
| Target | Example | Guard |
|---|---|---|
| **Global** (no `tenant_id` column) | `builtin_tools`, disk config, `pip`/`npm`/`apk` | HTTP `requireMasterScope` · WS `requireMasterScope(requireOwner(...))` |
| **Tenant-scoped** (has `tenant_id` column) | `agents`, `skills`, `llm_providers` | `requireTenantAdmin` + store SQL `WHERE tenant_id = $N` |
Shared predicate: `store.IsMasterScope(ctx)` (`internal/store/context.go`).
**Anti-patterns flagged in review:**
- `store.Update(...)` on a no-`tenant_id` table without a master-scope check upstream
- Write SQL with `WHERE ... (tenant_id = $N OR tenant_id IS NULL)` — the `IS NULL` arm lets tenants reach system rows
- `requireAuth(RoleAdmin)` as the **sole** gate on a global-state write
- Admin revoke/delete handlers that skip pre-fetch ownership verification (store SQL alone is not enough when it matches `IS NULL` arms)
### Commit Messages
Use conventional commits: