review-multi-tenancy
GitHub用于审查涉及多租户隔离的PR,检查实体继承、@Filter注解及v1/v2机制切换,确保数据隔离正确性。
Trigger Scenarios
Install
npx skills add OpenAEV-Platform/openaev --skill review-multi-tenancy -g -y
SKILL.md
Frontmatter
{
"name": "review-multi-tenancy",
"description": "Step-by-step tenant isolation audit for OpenAEV pull requests. Use when reviewing PRs that touch entities, repositories, native queries, or migrations."
}
Review Multi-Tenancy
Step 1 — Inventory changed entities
# List all entity classes modified in the PR
git diff --name-only HEAD~1 | grep -E "openaev-model/.*\.java$"
For each modified entity, determine:
- Does it extend
TenantBase? → tenant-scoped - Does it extend
Baseonly? → platform-level - Is it a new entity? → must decide scoping
Step 2 — Verify @Filter on tenant-scoped entities
grep -rn "extends TenantBase" --include="*.java" -l openaev-model/ | while read f; do
echo "=== $f ==="
grep -c "tenantFilter" "$f"
done
Expected: every TenantBase entity has @Filter(name = "tenantFilter").
Missing @Filter = 🔴 CRITICAL — Hibernate won't apply tenant filtering.
NOTE: if the API, entity class is using v2 API isolation mechanism, then the
@Filterannotation should not be there. Check.github/skills/activate-tenant-table/SKILL.md.
Step 2b — Determine v1 vs v2 for every touched table
Before auditing anything else, classify each entity/table touched by the PR.
The correct pattern (and the correct severity for a missing check) is
different depending on which mechanism governs the table — applying v1
expectations (@Filter, TenantContext) to a v2-active table, or vice versa,
produces false positives/negatives in every later step.
# v2-active tables: the authoritative list
grep -n "active-tables" openaev-api/src/main/resources/application.properties
# does the touched entity/repository carry TxCtx-based scoping anywhere in the diff?
grep -rn "TxCtx\|TenantScopedTransaction\|RequireTenantSelector\|can_access_tenant" --include="*.java" $(git diff --name-only HEAD~1) 2>/dev/null
# does it still rely on v1?
grep -rn "TenantBase\b\|@Filter(\"tenantFilter\")\|TenantContext\." --include="*.java" $(git diff --name-only HEAD~1) 2>/dev/null
Classify each touched table as one of:
- v2-active — its name appears in
openaev.tenant.active-tables. Isolation is enforced byTenantStatementInspector+can_access_tenant, scoped by aTxCtxparameter threaded through every@Transactionalentrypoint that reaches it (directly, via another service, or via an association/join — seeactivate-tenant-table/SKILL.mdPhase 1). For these tables:@Filter("tenantFilter")should be ABSENT (v1 and v2 are mutually exclusive per table; both present is a red flag, not defense in depth)- every new/changed
@Transactionalmethod whose call graph reaches the table must carry aTxCtxparam — missing it is 🔴 CRITICAL (silent empty-result regression, not a loud failure; see thechangeExerciseStatusincident: a caller of the same EE-executor gate as two already-wired sibling endpoints shipped withoutTxCtxand silently bypassed the license check) - write/attribution endpoints (composite-PK lookups, row creation) need
@RequireTenantSelectoron theTxCtxparam, not just its presence - a native
@QuerythatJOINs a v2-active table anywhere in the codebase (not just its own repository) is pulled into the fail-closed rewrite — check its FROM/JOIN shape againstTenantStatementInspectorTest(#7007)
- v1 (still
@Filter-based) — not inactive-tables. Isolation is Hibernate@Filter+TenantBaseListener, ambient viaTenantContext.getCurrentTenant(). For these tables, Steps 2-7 below (as originally written) apply as-is. - permanently v1 by design — a table deliberately kept off
active-tablesbecause its access pattern (e.g. a native upsert withON CONFLICT ... RETURNING) is a shape the inspector cannot rewrite. Every statement on it must carry its own explicit tenant predicate (WHERE tenant_id = :tenantId/tenant.id IN :tenantIds) since neither@Filternor the inspector protects it. Missing predicate here is 🔴 CRITICAL — there is no fallback mechanism at all for this class of table. Model:AttackPathGraphVersionRepository(see its class javadoc).
If the PR adds a TxCtx parameter but the table is NOT in active-tables,
that's a no-op today (the aspect sets an inert GUC) — not wrong, but flag it
as premature/dead code unless it's part of a deliberate pre-wiring pass, so it
doesn't get mistaken for the table having gone live.
Step 3 — Audit native queries
grep -rn "nativeQuery = true" --include="*.java" openaev-model/ openaev-api/
For each native query, first check Step 2b's classification of the table(s) it touches:
- table is v2-active → this is the #7007 FROM/JOIN-shape check, not the v1
checklist below: verify the shape is in
TenantStatementInspectorTest's accepted list (a table-function FROM item needsLATERAL); a refused shape fails closed (TENANT_FILTERING_REFUSED) in production, not silently - table is v1 or permanently-v1-by-design → the checklist below applies:
- Does it have
WHERE tenant_id = :tenantId? ✅ - Does it use SpEL:
WHERE tenant_id = :#{#tenantContext.currentTenant}? ✅ - Does it join through a tenant-filtered entity? ✅
- Neither? → 🟠 HIGH — filter bypass
- Does it have
Step 4 — Audit migrations
git diff --name-only HEAD~1 | grep "migration"
For each migration touching a tenant-scoped table, verify:
- ☐
tenant_id VARCHAR(255) NOT NULLcolumn - ☐ FK to
tenants(tenant_id) ON DELETE CASCADE - ☐ Index on
tenant_id - ☐ Unique constraints composite with
tenant_id
Step 5 — Audit async and background contexts
grep -rn "@Async\|@Scheduled\|CompletableFuture\|ExecutorService\|@EventListener" --include="*.java" openaev-api/src/main/java/ | grep -v "test"
# is this entry point already converted to the background transaction primitive?
grep -rln "TenantScopedTransaction" --include="*.java" openaev-api/src/main/java/
First check whether the job opens its transaction with @Transactional (v1-style, still on the frozen background baseline — see reduce-tx-baseline/SKILL.md) or with TenantScopedTransaction.execute/executeNew/forEachTenant (converted to the v2 background primitive, #6398). The expectations differ:
- Not yet converted (
@Transactional/ rawTransactionTemplate): flag it perreduce-tx-baseline/SKILL.md, not this step. - Converted (
TenantScopedTransaction): the scope IS carried correctly for every read against a v2-active table — that part needs noTenantContextat all, since reads go through theTxCtx/GUC/inspector path, not@Filter. The remaining question is whetherTenantContext.setCurrentTenant()inside the block is still doing real work or is leftover from before the conversion:- Does the block persist/save any
TenantBaseentity (.save(,.saveAll(,.persist(, or a service method that does)?TenantBaseListener.@PrePersiststampstenant_idfromTenantContext.getCurrentTenant()— this is v1 attribution machinery that v2 activation does not replace (v2 only changes how reads are filtered, not how inserts are attributed). If yes →TenantContext.setCurrentTenant()/clearCurrentTenant()(in afinally) is REQUIRED. Do not suggest removing it; flag its absence as 🔴 CRITICAL instead (rows would be attributed toTenant.DEFAULT_TENANT_UUIDon the ambient fallback). - Does the block call into any code that is not yet v2 (a legacy
io.openaev.restservice, an entity still governed by@Filter, or anything readingTenantContext.getCurrentTenant()directly)? If yes → keep it, same reasoning. - If the block is read-only and every table it touches is v2-active (cross-check against
openaev.tenant.active-tables, Step 2b) and it calls no legacy/v1 code path →TenantContext.setCurrentTenant()is redundant: theTxCtxpassed toTenantScopedTransaction.execute(...)already carries the scope the reads need. Flag it as 🟡 MEDIUM — suggestion to remove the dead v1 call, progressing the job fully onto v2, but call this out explicitly as a suggestion, not a blocker, and require the reviewer to confirm both bullets above before recommending removal (a false positive here reintroduces the DEFAULT_TENANT_UUID attribution bug).
- Does the block persist/save any
For a job NOT yet converted to TenantScopedTransaction (still async/@Scheduled raw):
- Is
TenantContext.setCurrentTenant()called before any DB access? - Is the tenant ID passed explicitly to the async method?
Step 6 — Audit API responses
grep -rn "tenantId\|tenant_id\|getTenant" --include="*.java" openaev-api/src/main/java/io/openaev/api/ openaev-api/src/main/java/io/openaev/rest/ | grep -v "@JsonIgnore" | grep -v "test"
Any tenant reference in API layer without @JsonIgnore = 🟠 HIGH.
Step 7 — Audit caching
grep -rn "@Cacheable\|@CachePut\|@CacheEvict" --include="*.java" openaev-api/src/main/java/
For each cached method:
- Does the cache key include tenant context?
- If not → 🟡 MEDIUM — cross-tenant cache poisoning risk
Step 8 — Audit dual-scope entities
Dual-scope entities (Settings, User, Role, Group) implement DualScopeBase and have nullable tenant_id:
# Check that dual-scope repos extend DualScopeRepository
grep -rn "DualScopeRepository\|DualScopeBase" --include="*.java" openaev-model/src/main/java/
For each dual-scope entity:
- ☐ Entity implements
DualScopeBase - ☐ Repository extends
DualScopeRepository(blocks unscopedfindAll(),findById()) - ☐ Two services:
PlatformXxxService+TenantXxxService - ☐ Two APIs:
PlatformXxxApi+TenantXxxApi - ☐ Repository only exposes scoped queries (
findByTenantIsNull,findByTenantId) - ☐
PlatformXxxServicenever receivestenantId;TenantXxxServicealways receivestenantId - Entity without
DualScopeBase= 🟠 HIGH - Repository without
DualScopeRepository= 🟠 HIGH - Single service mixing both scopes = 🟠 HIGH
Step 9 — Compile findings
Generate the Multi-Tenancy Review Summary following the output format
defined in multi-tenancy-reviewer.agent.md.
Version History
- 3.260818.1 Current 2026-08-20 12:00


