Skip to content

fix(organizations): enforce shared parent and child creation limit - #7940

Merged
eshurakov merged 4 commits into
mainfrom
eshurakov/fleet-aurora
Oct 18, 2026
Merged

eshurakov merged 4 commits into
mainfrom
eshurakov/fleet-aurora

Conversation

@eshurakov

@eshurakov eshurakov commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Enforce the approved five-active-organization limit across both self-service creation paths, regardless of payment status.
  • Move the quota check into the creation transaction and lock the user row before counting, preventing concurrent requests from the same user from bypassing the limit.
  • Count creator-attributed organizations, direct memberships, and inherited child organizations once, including top-level organizations created without an owner membership.
  • Restrict the staff exemption to the exact, case-insensitive kilocode.ai domain.
  • Preserve child organizations with NULL creator attribution, so existing bouncer and spend-alert behavior does not change.
  • Preserve trusted admin creation, existing role and trial checks, and replacement of deleted organizations.

Cause

The child-creation endpoint had no quota check. Top-level creation checked only direct memberships outside the creation transaction, allowing unassigned creations and concurrent requests to bypass the existing limit. The staff-domain suffix check also accepted unrelated domains such as notkilocode.ai.

Verification

  • Added 14 database-backed tRPC regression tests. Five bypass cases failed before the fix.
  • Eight related organization and export suites: 339 passed, 2 skipped.
  • pnpm --filter web typecheck passed; final review corrections also passed the web tsgo check.
  • pnpm --filter @kilocode/web-shared typecheck passed.
  • Changed-file oxlint, pnpm format, and git diff --check passed.
  • Independent plan, diff, and attribution reviews completed.
  • Initial CI fixture failures were corrected without weakening runtime enforcement. Admin fixtures no longer depend on the production staff exemption.

Targeted test command:

JEST_SILENT=true pnpm --filter web test --runInBand src/routers/organizations/organization-creation.test.ts src/routers/organizations/organization-router.test.ts src/routers/organizations/organization-admin-router.test.ts src/lib/organizations/organization-hierarchy.test.ts src/routers/user-exports-router.test.ts src/routers/organizations/organization-modes-router.test.ts src/routers/organizations/organization-settings-router.test.ts ../../packages/web-shared/src/lib/organizations/organizations.test.ts

Scope

No schema migration or new dependency. This controls future creation; it does not delete existing organizations or block their use. It is not an absolute membership/access cap: invitations, trusted admin changes, and another manager creating an inherited child can increase another user’s accessible total. A regression documents that sequential boundary and confirms subsequent creation is blocked. Full repository validation and production-scale query-plan measurements were not run locally. No production data was changed.

@kilo-code-bot

kilo-code-bot Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit restores NULL creator attribution for self-service child organizations and adds a regression covering the inherited-access boundary; both changed files are internally consistent and I found no new defects, and the previously flagged lock-scope concern is a documented, non-bypassable scope limit rather than a defect.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0
Files Reviewed (3 files)
  • apps/web/src/lib/organizations/organization-hierarchy.ts - no issues (removal of created_by_kilo_user_id: userId restores the pre-PR NULL attribution; userId is still used by the quota guard; children are still counted via parent_organization_id, so no quota hole)
  • apps/web/src/routers/organizations/organization-creation.test.ts - no issues (updated assertion matches the code change; the new regression's counts and rejections are consistent with the limit helper)
  • packages/web-shared/src/lib/organizations/organizations.ts - no issues (targeted re-verification of the prior line-258 finding; see notes)

Notes

  • Prior WARNING at packages/web-shared/src/lib/organizations/organizations.ts:258 (parent-creation path counts without the hierarchy advisory lock): not carried forward. The count can only be inflated by another actor's authorized child creation under a shared parent, which is itself quota-enforced; the proposed advisory lock cannot prevent it because the same end state is reachable with fully serialized operations. The PR description now explicitly scopes this as a per-acting-user creation quota that is "not an absolute membership/access cap", and a82e7400 adds a database-backed regression documenting the boundary. Same-user concurrency remains serialized by the kilocode_users row FOR UPDATE lock in enforceOrganizationCreationLimit.
  • Context checked (unchanged): organization-creation-limit.ts (lock + count logic), organization-router.ts (createChild used only via organizationAdminMutationProcedure, so the acting user is always in managedParents and stays counted).
  • No memory/resource concerns: no listeners, timers, caches, or unclosed handles were added; the advisory lock and row lock are transaction-scoped.
  • Full-repository typecheck/lint/tests were not run in this session; only static inspection of the changed lines and targeted reads were performed.
Previous Review Summaries (2 snapshots, latest commit b848f6a)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit b848f6a)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental commit is test-only and resolves both prior test-fixture suggestions; the remaining risk is the parent-creation path enforcing the shared quota without the hierarchy advisory lock used by child creation, leaving a narrow cross-user bypass.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
packages/web-shared/src/lib/organizations/organizations.ts 258 Count runs under the user row lock only; a second admin of a shared parent can add a child past the limit

SUGGESTION

File Line Issue
apps/web/src/lib/organizations/organization-hierarchy.ts 120 Child orgs now carry created_by_kilo_user_id, changing bouncer/payer attribution

Verified

  • Incremental commit b848f6accb touches only tests; it removes the always-true assertion in organization-creation.test.ts and switches the admin-router fixture to createTestOrganization with the non-staff @admin.example.com email, resolving both prior test suggestions.
  • Both remaining issues were re-verified against current HEAD: createOrganization still calls enforceOrganizationCreationLimit(tx, userId) without pg_advisory_xact_lock, and createChildOrganization still writes created_by_kilo_user_id: userId.
  • No new issues in the changed test lines: createOrganization is no longer imported or referenced in organization-admin-router.test.ts, createTestOrganization inserts exactly one owner membership (the helper calls createOrganization(name) without a user, so there is no double membership), and the re-read assertions in organization-creation.test.ts still prove persisted creator attribution.
  • No memory or resource leaks introduced: both SELECT ... FOR UPDATE and pg_advisory_xact_lock are transaction-scoped and released on commit/rollback; the test-only changes add no listeners, timers, or caches.
  • Existing inline comments on both remaining issues are still active at current HEAD and were not duplicated.

Notes

Full-repository typecheck/lint/tests were not run in this session, so the suite-level claims in the PR description (five organization suites, 77 tests) were not independently reproduced; only static inspection of the changed lines was performed.

Files Reviewed (4 files)
  • apps/web/src/routers/organizations/organization-admin-router.test.ts - no new issues
  • apps/web/src/routers/organizations/organization-creation.test.ts - no new issues
  • apps/web/src/lib/organizations/organization-hierarchy.ts - 1 issue (carried forward, re-verified)
  • packages/web-shared/src/lib/organizations/organizations.ts - 1 issue (carried forward, re-verified)

Fix these issues in Kilo Cloud

Previous review (commit f5d1d13)

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The core limit query and staff-domain tightening are correct, but the parent-creation path enforces the shared quota without the hierarchy advisory lock used by child creation, leaving a narrow cross-user bypass.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 3
Issue Details (click to expand)

WARNING

File Line Issue
packages/web-shared/src/lib/organizations/organizations.ts 258 Count runs under the user row lock only; a second admin of a shared parent can add a child past the limit

SUGGESTION

File Line Issue
apps/web/src/lib/organizations/organization-hierarchy.ts 120 Child orgs now carry created_by_kilo_user_id, changing bouncer/payer attribution
apps/web/src/routers/organizations/organization-admin-router.test.ts 41 Fixture now relies on the production @kilocode.ai staff exemption
apps/web/src/routers/organizations/organization-creation.test.ts 106 Assertion is satisfied by the helper's spread and cannot fail

Verified

  • Limit arithmetic is sound: limit(MAX) with length >= MAX blocks both exactly-at and above-limit accounts, and the single-table OR cannot produce duplicate rows.
  • Deleted organizations are excluded; the staff exemption correctly narrows from endsWith('kilocode.ai') to an exact, case-insensitive domain match, and reads the email from the DB rather than the session.
  • The check is inside the creation transaction before any insert or Stripe call, so a rejected creation has no partial side effects, and both self-service paths are covered; the admin path remains an explicit userId-less bypass.
  • No import cycle; TRPCError/@sentry/nextjs usage matches existing web-shared conventions and declared dependencies.
  • No memory or resource leaks introduced: both SELECT ... FOR UPDATE and pg_advisory_xact_lock are transaction-scoped and released on commit/rollback; no new listeners, timers, or caches.

Notes

Full-repository typecheck/lint/tests were not run in this session; the five organization suites cited by the PR were not executed. Other suites that call createOrganization with a real user id (for example organization-seats, organization-modes, kilo-pass-org/service) were inspected and appear to stay under the limit per user.

Files Reviewed (7 files)
  • apps/web/src/lib/organizations/organization-hierarchy.ts - 1 issue
  • apps/web/src/routers/organizations/organization-admin-router.test.ts - 1 issue
  • apps/web/src/routers/organizations/organization-creation.test.ts - 1 issue
  • apps/web/src/routers/organizations/organization-router.ts - no issues
  • packages/web-shared/src/lib/organizations/organization-creation-limit.ts - no issues
  • packages/web-shared/src/lib/organizations/organizations.ts - 1 issue
  • packages/web-shared/src/tests/helpers/organization.helper.ts - no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 51.8K · Output: 11.2K · Cached: 419.7K

Review guidance: REVIEW.md from base branch main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants