Skip to content

feat(admin): require scoped support grants for customer org access - #9618

Open
pandemicsyn wants to merge 25 commits into
feat/admin-worker-capabilityfrom
feat/admin-support-org-access
Open

pandemicsyn wants to merge 25 commits into
feat/admin-worker-capabilityfrom
feat/admin-support-org-access

Conversation

@pandemicsyn

@pandemicsyn pandemicsyn commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What changes

Part 4 of the admin lockdown stack. Ends automatic owner access to customer orgs, controlled by one switch.

  • SUPPORT_ACCESS_ENABLED unset (default): admin access works as today.
  • SUPPORT_ACCESS_ENABLED=true: an admin who isn't a member of an org needs a support grant for it, requested from /admin/support-access with a reason.
    • Read grants: active immediately, up to 60 minutes.
    • Write grants: approved by a different admin, up to 15 minutes, and they require a passkey check (feat(admin): require session-bound passkey step-up for admin access #9615).
    • Never available through support: API tokens, payments, integrations, members, SSO, data export, org deletion.
    • Admins who are real members keep their role. Every grant event is recorded permanently.

Always on: members no longer inherit access from a deleted parent org (bug fix).

Before merge / deploy

  • The new migration (support_access_grants, support_access_grant_events) runs automatically in the deploy pipeline.

Before turning it on

Verification

Automated tests only. A staging walkthrough is planned before turning it on.

Visual Changes

New /admin/support-access page and an org banner while a grant is active.

@pandemicsyn
pandemicsyn marked this pull request as ready for review November 2, 2026 20:06
@kilo-code-bot

kilo-code-bot Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 45b5a174f gates every support-grant lifecycle mutation on SUPPORT_ACCESS_ENABLED, makes the step-up adapter refuse missing/non-session credentials, and moves grant revocation into a shared transaction helper; the prior carry-over findings remain resolved at HEAD.

Assumptions: [INFERENCE] Automated tests were not executed in this read-only sandbox; findings were verified statically against HEAD (45b5a174f). revoke is intentionally left ungated (fail-closed direction); assertSupportAccessEnabled() now makes the inner isSupportAccessEnabled() check in approve redundant but harmless.

Files Reviewed (10 files)
  • apps/web/src/lib/admin/support-step-up.test.ts
  • apps/web/src/lib/user/index.ts
  • apps/web/src/routers/admin/admin-support-access-router.ts
  • apps/web/src/routers/admin/support-access-router.test.ts
  • packages/web-shared/src/lib/admin/support-access-resolver.ts
  • packages/web-shared/src/lib/admin/support-access.ts
  • packages/web-shared/src/lib/admin/support-step-up.ts
  • packages/web-shared/src/lib/trpc/transport.test.ts
  • packages/web-shared/src/lib/trpc/transport.ts
  • packages/web-shared/src/routers/organizations/utils.test.ts
Previous Review Summaries (7 snapshots, latest commit 2af43cb)

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

Previous review (commit 2af43cb)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 2af43cb3a correctly switches the support-access self-action guards to session.kiloUserId, so Deny and Mark reviewed now compare provider-independent Kilo user ids; the previously carried wouldPassWithoutAdmin finding is also resolved at HEAD.

Assumptions: [INFERENCE] Automated tests were not executed in this read-only sandbox; findings were verified statically against HEAD (2af43cb3a). The merge commit e522c6bd5 brought the base branch forward and resolved conflicts in the admin layout, admin router, privileged inventory, and audit-log files; those resolutions were checked and appear consistent (inventory occurrence counts match the files).

Verification Notes

  • SupportAccessContent.tsx:33 now reads session?.kiloUserId; the shared session callback (packages/web-shared/src/lib/user/server.ts:106) exposes it to the browser, and grants store ctx.user.id (admin-support-access-router.ts:71), so both guards at lines 359/367 and 481 compare the same id space.
  • packages/web-shared/src/routers/organizations/utils.ts:428,452 now record membershipObserved ? role !== null : null, resolving the earlier null-instead-of-false concern.
Files Reviewed (12 files)
  • apps/web/src/app/admin/support-access/SupportAccessContent.tsx
  • apps/web/src/app/admin/layout.tsx
  • apps/web/src/routers/admin-router.ts
  • apps/web/src/routers/admin/admin-support-access-router.ts
  • apps/web/src/lib/user/index.test.ts
  • apps/web/src/lib/user/next-auth-options.ts
  • packages/web-shared/src/lib/admin/privileged-inventory.ts
  • packages/web-shared/src/lib/admin/admin-access-log.ts
  • packages/web-shared/src/lib/admin/support-access.ts
  • packages/web-shared/src/lib/admin/support-access-resolver.ts
  • packages/web-shared/src/lib/user/server.ts
  • packages/web-shared/src/routers/organizations/utils.ts

Previous review (commit 1f2f0ee)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental commit correctly restores grant-class precedence and stops expired/revoked grants from masking real denial reasons, but the new self-deny/self-review UI guards compare against the auth-provider subject instead of the Kilo user id, so they stay inert for Google and magic-link admins.

Assumptions: [INFERENCE] Automated tests were not executed in this sandbox; findings were verified statically against current HEAD (1f2f0ee). The carried-forward audit issue already has an active inline comment from the previous review and is listed here only for completeness.

Overview

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

SUGGESTION

File Line Issue
apps/web/src/app/admin/support-access/SupportAccessContent.tsx 367 The new self-deny guard (and the self-review guard at line 481) compares session.user.id, which the session callback fills from token.sub (provider subject: Google sub, email-<address> for magic link), against grant.actorKiloUserId, which is kilocode_users.id. The comparison never matches for Google/email sign-ins, so the buttons stay enabled; use session?.kiloUserId. Server enforcement still rejects these actions, so this is UI-only.
packages/web-shared/src/routers/organizations/utils.ts 432 Carried forward (unresolved): both ON support branches record wouldPassWithoutAdmin: null, but they are only reachable after the rows.length > 0 check, where the membership lookup ran and the result is known to be false (same at line 456). null is reserved for "cannot measure".
Files Reviewed (9 files)
  • apps/web/src/app/admin/support-access/SupportAccessContent.tsx - 1 issue
  • apps/web/src/lib/organizations/organization-auth.test.ts
  • apps/web/src/lib/organizations/organization-auth.ts
  • apps/web/src/lib/security-agent/router/shared-handlers.ts
  • apps/web/src/routers/admin/support-access-router.test.ts
  • packages/web-shared/src/lib/admin/support-access-resolver.ts
  • packages/web-shared/src/lib/admin/support-access.ts
  • packages/web-shared/src/routers/organizations/utils.test.ts
  • packages/web-shared/src/routers/organizations/utils.ts - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit 28ac13e)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The binary on/off conversion is consistent across the access-resolution call sites, but resolveOrganizationAccess now records a wouldPassWithoutAdmin value it can actually measure and lets expired/revoked grants override the real denial reason.

Assumptions: [INFERENCE] Tests were not executed in this sandbox; the on/off mapping and inventory counts were verified statically against current HEAD.

Overview

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

SUGGESTION

File Line Issue
packages/web-shared/src/routers/organizations/utils.ts 403 The ON support branches record wouldPassWithoutAdmin: null, but they are only reachable when rows.length === 0 (the prior check throws), so the membership result is known to be false. The audit contract reserves null for "cannot measure".
packages/web-shared/src/routers/organizations/utils.ts 412 latestSupportGrantStatus now runs on every denial, and supportAccessReasonFor checks grantStatus first, so an old expired/revoked grant masks deny_not_allowlisted / deny_never_list / deny_action_class and misdirects the admin.
Files Reviewed (44 files)
  • ENVIRONMENT.md
  • apps/web/src/app/admin/components/AppSidebar.tsx
  • apps/web/src/app/admin/components/PlatformAdminsContent.tsx
  • apps/web/src/app/admin/layout.tsx
  • apps/web/src/app/admin/support-access/duration.ts
  • apps/web/src/components/organizations/OrganizationByPageLayout.tsx
  • apps/web/src/lib/admin/admin-permissions.ts
  • apps/web/src/lib/admin/support-access-service.ts
  • apps/web/src/lib/integrations/platforms/bitbucket/credentials.ts
  • apps/web/src/lib/integrations/platforms/bitbucket/workspace-access-token-organization-authorization.ts
  • apps/web/src/lib/organizations/organization-auth.test.ts
  • apps/web/src/lib/organizations/organization-auth.ts
  • apps/web/src/lib/security-agent/router/shared-handlers.ts
  • apps/web/src/lib/user/index.test.ts
  • apps/web/src/lib/user/index.ts
  • apps/web/src/routers/__snapshots__/privileged-inventory.test.ts.snap
  • apps/web/src/routers/admin-platform-admins.test.ts
  • apps/web/src/routers/admin-router.ts
  • apps/web/src/routers/admin/admin-support-access-router.ts
  • apps/web/src/routers/admin/support-access-router.test.ts
  • apps/web/src/routers/cli-sessions-v2-router.test.ts
  • apps/web/src/routers/cli-sessions-v2-router.ts
  • apps/web/src/routers/code-indexing/code-indexing-router.ts
  • apps/web/src/routers/github-apps-router.ts
  • apps/web/src/routers/organizations/organization-members-router.test.ts
  • apps/web/src/routers/organizations/organization-members-router.ts
  • apps/web/src/routers/organizations/organization-sub-organizations-router.ts
  • apps/web/src/routers/organizations/organization-support-access.test.ts
  • apps/web/src/routers/privileged-inventory.test.ts
  • apps/web/src/routers/root-router.test.ts
  • packages/db/src/migrations/0325_support_access_grants.sql
  • packages/db/src/migrations/meta/0325_snapshot.json
  • packages/db/src/migrations/meta/_journal.json
  • packages/db/src/schema.ts
  • packages/web-shared/src/lib/admin/privileged-access.test.ts
  • packages/web-shared/src/lib/admin/privileged-access.ts
  • packages/web-shared/src/lib/admin/privileged-inventory.ts
  • packages/web-shared/src/lib/admin/support-access-resolver.ts
  • packages/web-shared/src/lib/admin/support-access.test.ts
  • packages/web-shared/src/lib/admin/support-access.ts
  • packages/web-shared/src/lib/token.test.ts
  • packages/web-shared/src/routers/organizations/utils.test.ts
  • packages/web-shared/src/routers/organizations/utils.ts - 2 issues
  • packages/web-shared/src/tests/helpers/user.helper.ts

Fix these issues in Kilo Cloud

Previous review (commit 67e5946)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The observe-mode follow-up now derives the REST decision from the admin's real membership, but the same derivation also runs in off mode and changes the default deployment's telemetry and wouldBe response.

Assumptions: [INFERENCE] Tests were not executed in this sandbox. The off-mode regression was verified statically against resolveOrganizationAccess (which keeps null in off) and the existing off-mode test expectations.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/organizations/organization-auth.ts 113 membershipDecision is computed outside the mode === 'observe' guard, so off mode now emits supportDecision: 'member'/'deny_actor_permission' and returns a non-null access.wouldBe for admin members, contradicting the documented off-mode invariance and the tRPC path.

SUGGESTION

File Line Issue
apps/web/src/app/admin/support-access/duration.ts 3 The 60/15-minute caps are hard-coded instead of derived from the exported SUPPORT_READ_MAX_DURATION_SECONDS / SUPPORT_WRITE_MAX_DURATION_SECONDS policy constants, so the console can silently drift from server policy.
Files Reviewed (7 files)
  • apps/web/src/app/admin/support-access/SupportAccessContent.tsx
  • apps/web/src/app/admin/support-access/duration.test.ts
  • apps/web/src/app/admin/support-access/duration.ts - 1 issue
  • apps/web/src/lib/organizations/organization-auth.test.ts
  • apps/web/src/lib/organizations/organization-auth.ts - 1 issue
  • packages/web-shared/src/lib/admin/support-access-resolver.ts
  • packages/web-shared/src/routers/organizations/utils.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 9393ea3)

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental follow-ups fix the previous off/observe admin fast-path regression, but the observe-mode decision recorded for admin members is still not what enforcement would reach, and the emergency-issuance permission mismatch remains.

Assumptions: [INFERENCE] The enforce mode clamp is not exercised in deployments, so enforce-only branches were verified statically. Tests were not executed in this sandbox. The previous round's fixes for the deleted-parent membership fix, grant-lookup error tolerance, off-mode null telemetry, and lock ordering were re-verified as present.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/admin/support-access-service.ts 509 Emergency issuance requires only userIsSuperadmin, but findUsableGrant requires the actor to have can_request_support_org_access = true. A superadmin without that flag (the default) issues an active grant that every resolution rejects, and cancel is gated on supportAccessRequesterProcedure, so it cannot be cancelled from the console.

SUGGESTION

File Line Issue
apps/web/src/lib/organizations/organization-auth.ts 112 In observe, the fast path records a grant-based supportDecision/access.wouldBe for admins whose membership already satisfies roles, though enforcement resolves those users as member/UNAUTHORIZED without consulting a grant.
packages/web-shared/src/routers/organizations/utils.ts 720 The membership fast path returns before recording a kilo_admin_elevation for a platform admin who is also a real member, so observe telemetry loses that event for callers such as code-indexing.
apps/web/src/app/admin/support-access/SupportAccessContent.tsx 131 The duration input still submits NaN, zero, and negative values to the router's positive-integer schema, surfacing only an opaque validation toast.
Files Reviewed (11 files)
  • apps/web/src/app/api/internal/glanceable-agents-snapshot/route.test.ts
  • apps/web/src/lib/admin/support-access-service.ts - 1 issue
  • apps/web/src/lib/organizations/organization-auth.test.ts
  • apps/web/src/lib/organizations/organization-auth.ts - 1 issue
  • apps/web/src/routers/cli-sessions-v2-router.ts
  • apps/web/src/routers/organizations/organization-router.ts
  • packages/web-shared/src/lib/admin/privileged-inventory.ts
  • packages/web-shared/src/lib/admin/support-access-resolver.ts
  • packages/web-shared/src/lib/trpc/transport.test.ts
  • packages/web-shared/src/routers/organizations/utils.test.ts
  • packages/web-shared/src/routers/organizations/utils.ts - 2 issues

Fix these issues in Kilo Cloud

Previous review (commit c2b4841)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
apps/web/src/lib/organizations/organization-auth.ts 102 New membership fast path bypasses the mode-aware resolver: platform admins with a real membership get that role (and no kilo_admin_elevation) in off/observe, changing REST role checks such as /api/organizations/[id]/user-tokens and resource delegation.
apps/web/src/lib/admin/support-access-service.ts 508 Emergency issuance requires only superadmin while the resolver requires can_request_support_org_access; a superadmin without that permission issues a grant they cannot use. Still present from the previous review; fixed the raw 23505/stale-sweep portion but not the permission mismatch.

SUGGESTION

File Line Issue
packages/web-shared/src/routers/organizations/utils.ts 673 Membership fast path skips the observe-mode kilo_admin_elevation for admin members, so code-indexing fetches undercount the observe data.
apps/web/src/app/admin/support-access/SupportAccessContent.tsx 131 Duration input still accepts NaN/zero/negative values and submits them, yielding an opaque server validation error. Carried forward; existing comment still applies.
Files Reviewed (30 files)
  • apps/web/src/app/admin/support-access/SupportAccessContent.tsx - 1 issue
  • apps/web/src/app/api/internal/glanceable-agents-snapshot/route.ts
  • apps/web/src/app/api/organizations/[id]/defaults/route.test.ts
  • apps/web/src/app/api/organizations/[id]/guardrails/route.test.ts
  • apps/web/src/app/api/organizations/[id]/modes/route.test.ts
  • apps/web/src/lib/admin/support-access-service.ts - 1 issue
  • apps/web/src/lib/auth/resource-delegation.test.ts
  • apps/web/src/lib/organizations/organization-auth.test.ts
  • apps/web/src/lib/organizations/organization-auth.ts - 1 issue
  • apps/web/src/lib/user/index.test.ts
  • apps/web/src/routers/active-sessions-router.ts
  • apps/web/src/routers/admin/support-access-router.test.ts
  • apps/web/src/routers/github-apps-router.ts
  • apps/web/src/routers/organizations/organization-members-router.test.ts
  • apps/web/src/routers/organizations/organization-router.ts
  • apps/web/src/routers/organizations/organization-support-access.test.ts
  • apps/web/src/routers/privileged-inventory.test.ts
  • apps/web/src/routers/root-router.test.ts
  • packages/db/src/migrations/0325_support_access_grants.sql
  • packages/db/src/migrations/meta/0325_snapshot.json
  • packages/db/src/migrations/meta/_journal.json
  • packages/db/src/schema.ts
  • packages/web-shared/src/lib/admin/admin-access-log.test.ts
  • packages/web-shared/src/lib/admin/privileged-inventory.ts
  • packages/web-shared/src/lib/admin/support-access-resolver.ts
  • packages/web-shared/src/lib/admin/support-access.test.ts
  • packages/web-shared/src/lib/admin/support-access.ts
  • packages/web-shared/src/lib/trpc/transport.ts
  • packages/web-shared/src/routers/organizations/utils.test.ts
  • packages/web-shared/src/routers/organizations/utils.ts - 1 issue

Assumptions: [INFERENCE] Enforce mode remains clamped to observe in deployments, so enforce-only paths were verified statically. Tests were not executed in this sandbox; the privileged-inventory occurrence counts were verified by grep, and generated migration snapshot/journal files were treated as artifacts. Previous-round fixes for the inventory counts, route mocks, grant action-class lookup, unique-violation handling, observe telemetry, and delegated-workload provenance were re-verified as resolved.

Fix these issues in Kilo Cloud

Previous review (commit b2c161e)

Status: 8 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
packages/web-shared/src/lib/admin/privileged-inventory.ts 679 Stale occurrences: 1 for support-access-service.ts (0 is_admin reads) fails the privileged-inventory coverage test.
apps/web/src/app/api/organizations/[id]/defaults/route.ts 31 The three switched routes now call getAuthorizedOrgContextOrSupport, but their tests still auto-mock only getAuthorizedOrgContext, so the new export returns undefined and the tests throw.
packages/web-shared/src/lib/admin/support-access-resolver.ts 118 Grant lookup ignores requiredActionClass; a longer-lived read grant shadows an active write grant and denies legitimate support writes.
apps/web/src/lib/admin/support-access-service.ts 491 Emergency issuance has no stale sweep or unique-violation handling; re-issuing (or a lingering active write grant) raises PostgreSQL 23505 as a 500.
apps/web/src/lib/admin/support-access-service.ts 485 Emergency grants are usable only by actors with can_request_support_org_access, which issuance does not require; a superadmin without it gets a dead, uncancellable grant.
packages/web-shared/src/routers/organizations/utils.ts 299 Observe telemetry records wouldPassWithoutAdmin: false for admin members whose real role satisfies the request, regressing the previous observation.

SUGGESTION

File Line Issue
apps/web/src/lib/organizations/organization-auth.ts 74 delegatedWorkload provenance is no longer forwarded, so some delegated credentials classify as human_bearer in elevation telemetry.
apps/web/src/app/admin/support-access/SupportAccessContent.tsx 127 Duration input accepts NaN/zero/negative values and submits them, yielding an opaque server validation error.

Assumptions: [INFERENCE] getAuthorizedOrgContextOrSupport has no REST-write caller yet, so requiredActionClassFor deriving support_read from a non-mutation HTTP method remains a latent fail-open rather than a live bypass. enforce is clamped to observe in deployments, so enforce-only paths were verified statically and through the test seam. Generated migration snapshot/journal files were treated as generated artifacts.

Files Reviewed (53 files)
  • packages/web-shared/src/lib/admin/privileged-inventory.ts - 1 issue
  • apps/web/src/app/api/organizations/[id]/defaults/route.ts - 1 issue
  • packages/web-shared/src/lib/admin/support-access-resolver.ts - 1 issue
  • apps/web/src/lib/admin/support-access-service.ts - 2 issues
  • packages/web-shared/src/routers/organizations/utils.ts - 1 issue
  • apps/web/src/lib/organizations/organization-auth.ts - 1 issue
  • apps/web/src/app/admin/support-access/SupportAccessContent.tsx - 1 issue
  • Remaining changed files reviewed: support-access policy/allowlists, resolver and access-log, grant router and revocation, admin permission management, organization auth/routes/pages, bypass gating (cli-sessions, code-indexing, members, sub-orgs, Bitbucket, security-agent, user deletion), DB schema and migration, and their tests.

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 126.7K · Output: 16.3K · Cached: 1.9M

Review guidance: REVIEW.md from base branch feat/admin-worker-capability

@pandemicsyn
pandemicsyn changed the base branch from feat/admin-assurance-inventory to feat/admin-worker-capability November 3, 2026 17:34
@pandemicsyn
pandemicsyn added this pull request to stack #9714 November 3, 2026 17:37
@pandemicsyn pandemicsyn changed the title feat(admin): add explicit support organization access (observe mode) feat(admin): require scoped support grants for customer org access Nov 3, 2026
…to feat/admin-support-org-access

# Conflicts:
#	apps/web/src/app/admin/layout.tsx
#	apps/web/src/lib/user/index.test.ts
#	apps/web/src/routers/admin-platform-admins.test.ts
#	apps/web/src/routers/admin-router.ts
#	packages/db/src/migrations/meta/0325_snapshot.json
#	packages/db/src/migrations/meta/_journal.json
#	packages/db/src/schema-types.ts
#	packages/web-shared/src/lib/admin/admin-access-log.test.ts
#	packages/web-shared/src/lib/admin/admin-access-log.ts
#	packages/web-shared/src/lib/admin/privileged-inventory.ts
#	packages/web-shared/src/lib/trpc/transport.test.ts
…to feat/admin-support-org-access

# Conflicts:
#	ENVIRONMENT.md
#	packages/db/src/migrations/meta/0333_snapshot.json
#	packages/web-shared/src/routers/organizations/utils.ts

This branch has not been deployed

No deployments
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