feat(audit): expand multi-tenant dimension to 100% standard coverage#328
Open
jeffersonrodrigues92 wants to merge 1 commit intomainfrom
Open
feat(audit): expand multi-tenant dimension to 100% standard coverage#328jeffersonrodrigues92 wants to merge 1 commit intomainfrom
jeffersonrodrigues92 wants to merge 1 commit intomainfrom
Conversation
Agent 33 (Multi-Tenant Patterns Auditor) was only covering ~60% of the canonical multi-tenant model defined in multi-tenant.md. This expands coverage from 9 checks to 34 checks, aligned with every requirement in the standard. New HARD GATES: non-canonical file detection, lib-commons v4 version verification. New CRITICAL checks: auth-before-tenant route ordering, global middleware detection. New HIGH checks: canonical env var names, circuit breaker, service API key, backward compatibility test, RabbitMQ vhost isolation, M2M credential security. New conditional M2M section (checks 30-34) for services with targetServices. Generated-by: Claude AI-Model: claude-opus-4-6
Contributor
WalkthroughUpdated the Multi-Tenant (Dimension 33) auditor prompt to require auditing against the complete canonical multi-tenant standard. Expanded codebase search patterns, replaced reference snippets to reflect canonical implementation requirements, strengthened HARD GATE criteria, and enhanced the output format schema with additional reporting fields. Changes
✨ Finishing Touches⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@default/skills/production-readiness-audit/SKILL.md`:
- Around line 3447-3448: The non-canonical file detection currently uses overly
broad glob patterns '**/tenant/' and '**/multitenancy/' which flag benign files
(docs, examples, tests) and causes HARD GATE failures; update the rule to narrow
the patterns to only match canonical source locations (e.g.,
'**/src/**/tenant/**', '**/src/**/multitenancy/**' or explicit
resolver/middleware directories) and/or add exclusions for common non-production
folders ('**/test/**', '**/docs/**', '**/examples/**'), and change the
enforcement from HARD GATE to a WARNING for these patterns so that
non-production files do not yield score=0; apply the same adjustments where the
patterns are repeated (the second occurrence mirrored at the later block).
- Around line 3590-3591: The auditor currently validates presence of the listed
canonical env vars but omits APPLICATION_NAME; update the env-var validation
logic and documentation to include APPLICATION_NAME as a required canonical
variable (e.g., extend the canonical set that currently lists MULTI_TENANT_*
items to also include APPLICATION_NAME), ensure the check that enforces "All X
canonical env var names present" increments and enforces the new required count,
and update any rejection rules that flag non-canonical names so they still apply
while allowing APPLICATION_NAME as valid for Tenant Manager settings resolution.
- Around line 3584-3585: The HARD GATE text that mandates "organization_id +
ledger_id" on all queries is too rigid; update the SKILL.md entries that list
the HARD GATEs (the lines referencing organization_id + ledger_id and
TenantMiddleware/MultiPoolMiddleware) to require that queries include tenant
scoping semantics instead of a specific key pair, and state that tenant
isolation must be validated via the canonical tenant resolution pattern
(referencing tenant-manager/core resolution) so modules may use their
module-specific scoping keys as long as TenantMiddleware or MultiPoolMiddleware
injects and enforces the tenant context.
- Around line 3633-3634: The conditional "only if service has targetServices" is
underspecified—update the SKILL.md section for "M2M credential provider" to
define a deterministic activation rule: require either (a) the service manifest
contains a non-empty targetServices array, or (b) an explicit boolean
flag/annotation (e.g., enableM2MCredentials=true) in service metadata, and
include the precedence rule if both exist; add a one-line detection algorithm
and an example manifest showing the field/annotation and expected behavior so
automated audits can consistently decide whether to enable the two-level cache
(L1 sync.Map 30s → L2 Redis → AWS Secrets Manager).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a643003-0a39-456c-a91b-ae904188b44b
📒 Files selected for processing (1)
default/skills/production-readiness-audit/SKILL.md
Comment on lines
+3447
to
+3448
| - Non-canonical: `**/tenant/`, `**/multitenancy/`, custom resolvers/middleware outside canonical paths | ||
| - go.mod: lib-commons version, lib-auth version |
Contributor
There was a problem hiding this comment.
Non-canonical file detection is too broad for HARD GATE use
The current path patterns are broad (**/tenant/, **/multitenancy/) and can misclassify docs/examples/tests as violations, causing incorrect score=0 outcomes.
Suggested adjustment
-- Non-canonical: `**/tenant/`, `**/multitenancy/`, custom resolvers/middleware outside canonical paths
+- Non-canonical: source implementation files that define custom tenant resolvers/middleware/pool managers
+ outside canonical lib-commons integration paths (exclude docs, tests, fixtures, vendored code)Also applies to: 3586-3587
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@default/skills/production-readiness-audit/SKILL.md` around lines 3447 - 3448,
The non-canonical file detection currently uses overly broad glob patterns
'**/tenant/' and '**/multitenancy/' which flag benign files (docs, examples,
tests) and causes HARD GATE failures; update the rule to narrow the patterns to
only match canonical source locations (e.g., '**/src/**/tenant/**',
'**/src/**/multitenancy/**' or explicit resolver/middleware directories) and/or
add exclusions for common non-production folders ('**/test/**', '**/docs/**',
'**/examples/**'), and change the enforcement from HARD GATE to a WARNING for
these patterns so that non-production files do not yield score=0; apply the same
adjustments where the patterns are repeated (the second occurrence mirrored at
the later block).
Comment on lines
+3584
to
+3585
| 2. (HARD GATE) All database queries include entity scoping (organization_id + ledger_id) within tenant database | ||
| 3. (HARD GATE) TenantMiddleware or MultiPoolMiddleware from lib-commons v4 injects tenant into request context |
Contributor
There was a problem hiding this comment.
Over-constrained HARD GATE for query scoping
Requiring organization_id + ledger_id on all queries is too strict and can create false HARD GATE failures for modules/entities that use different domain scoping keys while still being tenant-isolated.
Suggested adjustment
-2. (HARD GATE) All database queries include entity scoping (organization_id + ledger_id) within tenant database
+2. (HARD GATE) All database queries include tenant-safe entity scoping within tenant database
+ (e.g., organization_id + ledger_id where applicable, or module-specific ownership keys defined by the bounded context)Based on learnings: dev-team/docs/standards/golang/multi-tenant.md:28-50 requires canonical tenant-manager/core resolution patterns but does not mandate a single universal pair of entity keys for every module.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@default/skills/production-readiness-audit/SKILL.md` around lines 3584 - 3585,
The HARD GATE text that mandates "organization_id + ledger_id" on all queries is
too rigid; update the SKILL.md entries that list the HARD GATEs (the lines
referencing organization_id + ledger_id and
TenantMiddleware/MultiPoolMiddleware) to require that queries include tenant
scoping semantics instead of a specific key pair, and state that tenant
isolation must be validated via the canonical tenant resolution pattern
(referencing tenant-manager/core resolution) so modules may use their
module-specific scoping keys as long as TenantMiddleware or MultiPoolMiddleware
injects and enforces the tenant context.
Comment on lines
+3590
to
+3591
| 6. All 8 canonical env var names present with exact names: MULTI_TENANT_ENABLED, MULTI_TENANT_URL, MULTI_TENANT_ENVIRONMENT, MULTI_TENANT_MAX_TENANT_POOLS, MULTI_TENANT_IDLE_TIMEOUT_SEC, MULTI_TENANT_CIRCUIT_BREAKER_THRESHOLD, MULTI_TENANT_CIRCUIT_BREAKER_TIMEOUT_SEC, MULTI_TENANT_SERVICE_API_KEY | ||
| 7. No non-canonical env var names for tenant configuration (e.g., TENANT_MANAGER_URL, TENANT_ENABLED are violations) |
Contributor
There was a problem hiding this comment.
Missing canonical APPLICATION_NAME validation in env-var checks
The canonical env checks exclude APPLICATION_NAME, so this auditor can mark a service compliant while missing a required variable for Tenant Manager settings resolution.
Suggested adjustment
-6. All 8 canonical env var names present with exact names: MULTI_TENANT_ENABLED, MULTI_TENANT_URL, MULTI_TENANT_ENVIRONMENT, MULTI_TENANT_MAX_TENANT_POOLS, MULTI_TENANT_IDLE_TIMEOUT_SEC, MULTI_TENANT_CIRCUIT_BREAKER_THRESHOLD, MULTI_TENANT_CIRCUIT_BREAKER_TIMEOUT_SEC, MULTI_TENANT_SERVICE_API_KEY
+6. Canonical env vars present with exact names, including required service identity variable:
+ APPLICATION_NAME, MULTI_TENANT_ENABLED, MULTI_TENANT_URL, MULTI_TENANT_ENVIRONMENT,
+ MULTI_TENANT_MAX_TENANT_POOLS, MULTI_TENANT_IDLE_TIMEOUT_SEC,
+ MULTI_TENANT_CIRCUIT_BREAKER_THRESHOLD, MULTI_TENANT_CIRCUIT_BREAKER_TIMEOUT_SEC,
+ MULTI_TENANT_SERVICE_API_KEYBased on learnings: dev-team/docs/standards/golang/multi-tenant.md:144-172 defines APPLICATION_NAME as a required environment variable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@default/skills/production-readiness-audit/SKILL.md` around lines 3590 - 3591,
The auditor currently validates presence of the listed canonical env vars but
omits APPLICATION_NAME; update the env-var validation logic and documentation to
include APPLICATION_NAME as a required canonical variable (e.g., extend the
canonical set that currently lists MULTI_TENANT_* items to also include
APPLICATION_NAME), ensure the check that enforces "All X canonical env var names
present" increments and enforces the new required count, and update any
rejection rules that flag non-canonical names so they still apply while allowing
APPLICATION_NAME as valid for Tenant Manager settings resolution.
Comment on lines
+3633
to
+3634
| M2M Credentials (CONDITIONAL — only if service has targetServices): | ||
| 30. M2M credential provider with two-level cache: L1 (sync.Map, 30s) → L2 (Redis) → AWS Secrets Manager |
Contributor
There was a problem hiding this comment.
M2M conditional checks need deterministic activation criteria
The section says “only if service has targetServices” but doesn’t define how to detect that. Without explicit trigger rules, audit outcomes will be inconsistent across runs/services.
Suggested adjustment
-M2M Credentials (CONDITIONAL — only if service has targetServices):
+M2M Credentials (CONDITIONAL — only if service has targetServices):
+Activation criteria (all required):
+1) Detect `targetServices` key in typed config/DTO/module wiring, and
+2) Detect outbound service-to-service credential provider usage in code paths.
+If not detected, mark M2M section as N/A and do not deduct score.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@default/skills/production-readiness-audit/SKILL.md` around lines 3633 - 3634,
The conditional "only if service has targetServices" is underspecified—update
the SKILL.md section for "M2M credential provider" to define a deterministic
activation rule: require either (a) the service manifest contains a non-empty
targetServices array, or (b) an explicit boolean flag/annotation (e.g.,
enableM2MCredentials=true) in service metadata, and include the precedence rule
if both exist; add a one-line detection algorithm and an example manifest
showing the field/annotation and expected behavior so automated audits can
consistently decide whether to enable the two-level cache (L1 sync.Map 30s → L2
Redis → AWS Secrets Manager).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
production-readiness-auditfrom 9 checks to 34 checks, achieving full alignment with the canonical multi-tenant model defined inmulti-tenant.mdGap analysis that motivated this change
Test plan
🤖 Generated with Claude Code