feat(admin): close worker admin support access with scoped capabilities by pandemicsyn · Pull Request #9616 · Kilo-Org/cloud

pandemicsyn · 2026-11-02T19:24:54Z

What changes

Part 3 of the admin lockdown stack. Replaces the forgeable isAdmin flag that Gastown and Wasteland use for admin support access with a short-lived (≤ 5 min), signed, per-town capability that web checks live on every use. It ships dormant, so admin access works as today until the switches below are flipped.

Services changed: apps/web, services/gastown, services/wasteland. No other Worker changes.

Always on: Workers refuse legacy admin tokens with no expiry or lasting more than 1 hour. Nothing in production issues those.

Before merge / deploy

Generate an ES256 key pair. Set web secret WORKER_ADMIN_CAPABILITY_PRIVATE_JWK and Worker var WORKER_ADMIN_CAPABILITY_PUBLIC_KEYS on Gastown and Wasteland. Either can stay empty until you enable capabilities.
Confirm Gastown can read the existing INTERNAL_API_SECRET Secrets Store entry (Wasteland already does).
Deploy web, then Gastown and Wasteland, with defaults (LEGACY_ADMIN_CLAIM_MODE=honor, capability issuance off).

To close the gap later (config only, needs #9615 live)

Web: WORKER_ADMIN_CAPABILITY_ISSUANCE=on.
Web: WORKER_ADMIN_LEGACY_CLAIM_ISSUANCE=off.
About 65 minutes later, Workers: LEGACY_ADMIN_CLAIM_MODE=reject.

Rollback: Workers back to honor, then legacy issuance back on.

Verification

Automated tests only (worker-utils, Gastown unit and integration, Wasteland, web).

Visual Changes

N/A

…ites

…into feat/admin-worker-capability

kilo-code-bot · 2026-11-02T20:22:59Z

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The follow-up commit correctly records a denied admin.capability / admin.legacy_claim decision for cross-user user-target town requests, and rewrites the cache-expiry test so it genuinely re-mints past the stored expiry; both previously reported suggestions are resolved and the incremental changes introduce no new issues.

Assumptions: [INFERENCE] Tests were not executed in this read-only review. The legacy-claim branch of the new denial audit (hasCapability=false, legacyClaim=true) has no in-repo caller of GET /api/users/:userId/towns/:townId/events, so its production reachability is inferred from the middleware flow.

Files Reviewed (3 files)

services/gastown/src/middleware/town-auth.middleware.ts
services/gastown/test/unit/town-auth.test.ts
apps/web/src/lib/worker-admin/browser-capability.test.ts

Previous Review Summaries (6 snapshots, latest commit 3122785)

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

Previous review (commit `3122785`)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The new user-target town binding correctly closes the cross-user events read, but it denies before the admin decision, so blocked support attempts leave no admin-audit row; the cached-capability expiry regression test is still vacuous.

Assumptions: [INFERENCE] No in-repo caller sends an admin capability to GET /api/users/:userId/towns/:townId/events (neither the admin REST client nor the browser capability client targets that route), so the audit gap is inferred from the middleware flow. Tests were not executed in this read-only review.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	2

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
`services/gastown/src/middleware/town-auth.middleware.ts`	97	The new user-target gate returns 403 before `decide()`, so a capability or legacy-claim request that names a town owned by another user is denied with no `admin.capability` / `admin.legacy_claim` row. Record the denied decision before returning.
`apps/web/src/lib/worker-admin/browser-capability.test.ts`	116	Carried forward: the first response expires in ~25s, below the 30s cache safety pad, so it is never cached and the test passes without the `cached.expiresAt > Date.now()` guard it is meant to protect. Make the first response cacheable and advance time past the cached expiry between calls.

Files Reviewed (3 files)

services/gastown/src/middleware/town-auth.middleware.ts - 1 issue
services/gastown/test/unit/town-auth.test.ts - no issues
apps/web/src/lib/worker-admin/browser-capability.test.ts - 1 issue (unchanged since the prior review; targeted re-check)

Fix these issues in Kilo Cloud

Previous review (commit `dfaff1d`)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The three prior findings are fixed in dfaff1d3 (target-kind derivation for user-scoped town routes, expiry re-check on cached capabilities, bounded introspection cache), but the new regression test for the capability expiry fix is vacuous because it never populates the cache.

Assumptions: [INFERENCE] No active in-repo caller of GET /api/users/:userId/towns/:townId/events was exercised live; correctness of the user-target path is verified from userAuthMiddleware, capabilityTargetKind, and the added unit test only. The bounded introspection cache's runtime behavior (500-entry cap, oldest-entry eviction) was reviewed statically; tests were not executed in this read-only review.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
`apps/web/src/lib/worker-admin/browser-capability.test.ts`	116	The "re-mints instead of returning an expired cached capability" test sets `capabilityExpiresAt` to ~25s, below the 30s cache safety pad, so the entry is never cached and the test passes without the `cached.expiresAt > Date.now()` guard. Make the first response cacheable and advance time past the cached expiry before the second call to actually exercise the regression.

Files Reviewed (6 files)

apps/web/src/lib/worker-admin/browser-capability.ts - fix verified (expiry re-checked before returning a cached capability)
apps/web/src/lib/worker-admin/browser-capability.test.ts - 1 issue
packages/worker-utils/src/worker-admin-capability.ts - fix verified (prunes expired entries and caps the cache at 500)
packages/worker-utils/src/worker-admin-capability.test.ts
services/gastown/src/middleware/town-auth.middleware.ts - fix verified (target kind derived from the matched allowlist action)
services/gastown/test/unit/town-auth.test.ts

Fix these issues in Kilo Cloud

Previous review (commit `ec160d8`)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental fixes close the legacy-elevation audit gap and the in-flight capability widening, but the cache loop can return an entry it has already deleted after expiry, and the town route still hard-codes a town target for user-scoped actions.

Assumptions: [INFERENCE] Deployment follows the documented order, so WORKER_ADMIN_LEGACY_CLAIM_ISSUANCE stays on between deploy steps 3 and 5 while capability issuance is on. No in-repo caller of GET /api/users/:userId/towns/:townId/events was found, so the target-kind mismatch is currently latent for the web admin REST client. Rig-bypass decisions where the owner rig is missing, or where introspection is unavailable, emit no admin.* event; that is not counted as a finding here. The admin_access sink retention/query behavior could not be exercised in this read-only review.

Overview

Severity	Count
CRITICAL	0
WARNING	2
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
`services/gastown/src/middleware/town-auth.middleware.ts`	32	Decisions always use a `town` target, but `GET /api/users/:userId/towns/:townId/events` is allowlisted as a user-target action and is guarded by this middleware (`gastown.worker.ts:1039`). `userAuthMiddleware` accepts the user-target capability, then this middleware re-decides against the town id and returns `target_mismatch`, so `reject` mode has no working admin path for that route.
`apps/web/src/lib/worker-admin/browser-capability.ts`	96	The expired entry deleted on line 95 is still returned here because the coverage check does not re-test `expiresAt`. Since `expiresAt` is padded 30s before real expiry, a batch can be sent with an already-expired capability and fail Worker verification instead of re-minting.

SUGGESTION

File	Line	Issue
`packages/worker-utils/src/worker-admin-capability.ts`	297	The read-introspection cache still evicts only the key being read. Since every capability carries a unique `jti`, entries whose keys are never read again accumulate for the isolate lifetime in both Workers. (Carried forward; unchanged in this increment.)

Files Reviewed (18 files)

apps/web/src/app/api/gastown/token/route.ts
apps/web/src/app/api/gastown/token/route.test.ts
apps/web/src/app/api/wasteland/token/route.ts
apps/web/src/app/api/wasteland/token/route.test.ts
apps/web/src/lib/worker-admin/browser-capability.ts - 1 issue
apps/web/src/lib/worker-admin/browser-capability.test.ts
packages/worker-utils/package.json
packages/worker-utils/src/worker-admin-actions.ts
packages/worker-utils/src/worker-admin-capability.ts - 1 issue (carried forward)
services/gastown/src/gastown.worker.ts
services/gastown/src/middleware/admin-audit.middleware.ts (deleted)
services/gastown/src/middleware/auth.middleware.ts
services/gastown/src/middleware/town-auth.middleware.ts - 1 issue
services/gastown/src/trpc/router.ts
services/gastown/test/unit/admin-access.test.ts (deleted)
services/gastown/test/unit/town-auth.test.ts
services/gastown/test/unit/worker-admin-procedures.test.ts
services/wasteland/src/trpc/worker-admin-procedures.test.ts

Previous review (commit `0db16a6`)

Status: 5 Issues Found | Recommendation: Address before merge

Executive Summary

The highest-risk gap remains in the capability mint path: a capability request still returns a one-hour legacy isAdmin identity token without recording that elevation, and two new scoping/telemetry changes can deny or hide legitimate admin support access.

Assumptions: [INFERENCE] Deployment follows the documented order, so WORKER_ADMIN_LEGACY_CLAIM_ISSUANCE stays on between deploy steps 3 and 5 while capability issuance is on. No in-repo caller of GET /api/users/:userId/towns/:townId/events was found, so the target-kind mismatch is currently latent for the web admin REST client. The admin_access sink retention/query behavior could not be exercised in this read-only review.

Overview

Severity	Count
CRITICAL	0
WARNING	3
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
`apps/web/src/app/api/gastown/token/route.ts`	60	A capability request still mints the identity token with `isAdmin: true` while legacy issuance is on, but `recordKiloAdminElevationForRequest` is skipped for that mint. Legacy admin elevations therefore stop being audited once capability issuance is enabled. Same pattern at `apps/web/src/app/api/wasteland/token/route.ts:51`.
`services/gastown/src/middleware/town-auth.middleware.ts`	48	Decisions always use a `town` target, but `GET /api/users/:userId/towns/:townId/events` is allowlisted as a `user`-target action and is guarded by this middleware (`gastown.worker.ts:1041`). A user-target capability always returns `target_kind_mismatch`, so admin support access to that route cannot use the capability path, and `reject` mode has no working admin path.
`apps/web/src/lib/worker-admin/browser-capability.ts`	137	`inflight` is keyed by target only. A concurrent request for the same town with a different action set joins an existing mint and returns a capability scoped to the first caller's actions, so its tRPC batch gets 403 until a later call re-mints.

SUGGESTION

File	Line	Issue
`services/gastown/src/middleware/admin-audit.middleware.ts`	15	Denied admin support attempts are no longer audited on town routes: `workerAdminDecision` is only set after the bypass is allowed, so an out-of-scope capability or rejected legacy claim emits no `admin.*` event.
`packages/worker-utils/src/worker-admin-capability.ts`	386	The read-introspection cache still evicts only the key being read. Since every capability carries a unique `jti`, entries whose keys are never read again accumulate for the isolate lifetime in both Workers.

Files Reviewed (26 changed files + 1 targeted re-check)

apps/web/src/app/api/gastown/token/route.ts - 1 issue
apps/web/src/app/api/gastown/token/route.test.ts
apps/web/src/app/api/gastown/token/route.real-guard.test.ts
apps/web/src/app/api/wasteland/token/route.ts - 1 issue (same pattern as Gastown)
apps/web/src/app/api/wasteland/token/route.test.ts
apps/web/src/lib/worker-admin/browser-capability.ts - 1 issue
apps/web/src/lib/worker-admin/browser-capability.test.ts
services/gastown/src/middleware/town-auth.middleware.ts - 1 issue
services/gastown/src/middleware/admin-audit.middleware.ts - 1 issue
services/gastown/src/middleware/auth.middleware.ts
services/gastown/src/gastown.worker.ts
services/gastown/scripts/patch-worker-types.mjs
services/gastown/test/unit/admin-access.test.ts
services/gastown/test/unit/worker-admin-procedures.test.ts
services/gastown/test/unit/worker-admin.test.ts
services/gastown/worker-configuration.d.ts (generated)
services/wasteland/src/wasteland.worker.ts
services/wasteland/src/trpc/init.ts
services/wasteland/src/trpc/ownership.ts
services/wasteland/src/trpc/worker-admin-procedures.test.ts
services/wasteland/src/trpc/create-upstream-credential.test.ts
services/wasteland/src/middleware/worker-admin.middleware.ts
services/wasteland/src/middleware/worker-admin.test.ts
services/wasteland/src/middleware/kilo-auth.middleware.test.ts
services/wasteland/scripts/patch-worker-types.mjs
services/wasteland/worker-configuration.d.ts (generated)
packages/worker-utils/src/worker-admin-capability.ts - 1 issue (unchanged; targeted re-check)

Fix these issues in Kilo Cloud

Previous review (commit `22f1d21`)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental changes tighten capability scope and minting, but the capability-token path now mints an unscoped legacy admin token without an admin-access audit record, and the previously reported town-target mismatch in townAuthMiddleware is still present.

Assumptions: [INFERENCE] Deployment follows the documented order, so WORKER_ADMIN_LEGACY_CLAIM_ISSUANCE stays on while WORKER_ADMIN_CAPABILITY_ISSUANCE is on; the admin_access sink's retention/query behavior could not be exercised in this read-only review.

Overview

Severity	Count
CRITICAL	0
WARNING	2
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
`apps/web/src/app/api/gastown/token/route.ts`	60	A capability request still mints the identity token with `isAdmin: true` while legacy issuance is on, but `recordKiloAdminElevationForRequest` is now skipped for that mint. Legacy admin elevations therefore stop being audited once capability issuance is enabled. Same pattern at `apps/web/src/app/api/wasteland/token/route.ts:51`.
`services/gastown/src/middleware/town-auth.middleware.ts`	28	Decisions always use a `town` target, but `GET /api/users/:userId/towns/:townId/events` is allowlisted as a `user`-target action and is also guarded by this middleware. A user-target capability always returns `target_mismatch`, so admin support access to that route can never use the capability path. Still unresolved.

SUGGESTION

File	Line	Issue
`packages/worker-utils/src/worker-admin-capability.ts`	386	The read-introspection cache still evicts only the key being read. Since every capability carries a unique `jti`, entries whose keys are never read again accumulate for the isolate lifetime in both Workers. Sweep expired entries or cap the map size in `set`. Still unresolved.

Files Reviewed (31 files)

apps/web/src/app/api/gastown/token/route.ts - 1 issue
apps/web/src/app/api/gastown/token/route.test.ts
apps/web/src/app/api/wasteland/token/route.ts - 1 issue (same pattern)
apps/web/src/lib/gastown/trpc.ts
apps/web/src/lib/wasteland/trpc.ts
apps/web/src/lib/worker-admin/browser-capability.ts
apps/web/src/lib/worker-admin/browser-capability.test.ts
apps/web/src/lib/worker-admin/actor-guard.test.ts
apps/web/src/routers/admin/gastown-router.ts
apps/web/src/routers/admin/gastown-router.capability.test.ts
packages/web-shared/src/lib/admin/privileged-inventory.ts
packages/web-shared/src/lib/config.server.ts
packages/worker-utils/src/index.ts
packages/worker-utils/src/worker-admin-capability.ts - 1 issue
packages/worker-utils/src/worker-admin-capability.test.ts
services/gastown/src/middleware/admin-audit.middleware.ts
services/gastown/src/middleware/gastown-access.middleware.ts
services/gastown/src/middleware/worker-admin.middleware.ts
services/gastown/src/middleware/town-auth.middleware.ts - 1 issue (targeted re-check)
services/gastown/src/trpc/router.ts
services/gastown/test/unit/admin-access.test.ts
services/gastown/test/unit/worker-admin.test.ts
services/gastown/worker-configuration.d.ts (generated)
services/wasteland/package.json
services/wasteland/scripts/patch-worker-types.mjs
services/wasteland/src/middleware/worker-admin.middleware.ts
services/wasteland/src/middleware/worker-admin.test.ts
services/wasteland/src/trpc/init.ts
services/wasteland/src/trpc/ownership.ts
services/wasteland/src/trpc/ownership.test.ts
services/wasteland/src/util/analytics.util.ts
services/wasteland/worker-configuration.d.ts (generated)

Previous review (commit `58f522a`)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

Admin capability access is silently broken for the user-scoped town-events route, and the Worker read-introspection cache grows without bound over an isolate's lifetime.

Assumptions: [INFERENCE] Worker deploy/rollback configuration (LEGACY_ADMIN_CLAIM_MODE, WORKER_ADMIN_CAPABILITY_PUBLIC_KEYS, INTERNAL_API_SECRET) matches the PR's documented activation order; those deployment settings could not be exercised in this read-only review.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

WARNING

| File | Line | Issue |
| ----------------

[Snapshot truncated.]

Additional previous summary content was truncated to keep this comment within platform limits.

_{Reviewed by deepseek-flash-latest · Input: 62.4K · Output: 12.4K · Cached: 775.4K}

_{Review guidance: REVIEW.md from base branch feat/admin-mfa-step-up}

…targets

… access

…sions

…rift

…rgets

…t/admin-worker-capability # Conflicts: # packages/web-shared/src/lib/admin/privileged-inventory.ts

pandemicsyn added 11 commits November 2, 2026 13:18

feat(worker-utils): add worker admin capability contract

5823c39

feat(gastown): read worker admin capabilities and harden legacy claims

944eee6

feat(wasteland): read worker admin capabilities and harden legacy claims

43ffebf

feat(admin): mint and introspect scoped worker admin capabilities

87a53ae

refactor(admin): drop dead isAdmin delegation path and classify new s…

c7e1e0f

…ites

feat(admin): issue per-procedure capability for worker support calls

65eabfa

Merge remote-tracking branch 'origin/feat/admin-assurance-inventory' …

53df3d9

…into feat/admin-worker-capability

fix(admin): report why a hardened legacy claim was rejected

b34e4e1

fix(admin): reject invalid capability headers on workers

41fc635

feat(admin): emit tRPC worker admin telemetry

515695a

refactor(admin): clamp capability ttl to the browser bound

58f522a

pandemicsyn marked this pull request as ready for review November 2, 2026 20:06

kilo-code-bot Bot reviewed Nov 2, 2026

View reviewed changes

Comment thread services/gastown/src/middleware/town-auth.middleware.ts Resolved

Comment thread packages/worker-utils/src/worker-admin-capability.ts Resolved

pandemicsyn added 4 commits November 2, 2026 14:38

fix(worker-utils): make capability module portable for all typecheck …

a212f39

…targets

fix(gastown): bind admin bypass to the named town and gate capability…

0b873a8

… access

fix(wasteland): align worker admin telemetry and cover ownership deci…

3b60e94

…sions

fix(admin): require a session step-up before minting worker capabilities

22f1d21

kilo-code-bot Bot reviewed Nov 2, 2026

View reviewed changes

Comment thread apps/web/src/app/api/gastown/token/route.ts Resolved

pandemicsyn added 6 commits November 2, 2026 15:01

test(gastown): exercise real procedures and audit only used bypasses

e0ee250

test(wasteland): cover real admin procedures and align telemetry

b17f9cf

fix(admin): reuse scoped browser capabilities and refuse guard user d…

1a4441a

…rift

chore(workers): widen generated admin env types

68a7616

test(wasteland): cover owner-only access gate

b29f4b7

test(wasteland): type the org member fixture

0db16a6

kilo-code-bot Bot reviewed Nov 2, 2026

View reviewed changes

Comment thread services/gastown/src/middleware/town-auth.middleware.ts Resolved

Comment thread apps/web/src/lib/worker-admin/browser-capability.ts Resolved

Comment thread services/gastown/src/middleware/admin-audit.middleware.ts Resolved

pandemicsyn added 4 commits November 2, 2026 15:21

refactor(worker-utils): split the client-safe admin action allowlist

5b4d76b

fix(gastown): restore admin parity and audit town denials once

f0d4839

test(wasteland): cover service-scope rejection

703a6cb

fix(admin): filter capability actions and widen concurrent requests

ec160d8

kilo-code-bot Bot reviewed Nov 2, 2026

View reviewed changes

Comment thread services/gastown/src/middleware/town-auth.middleware.ts Resolved

Comment thread apps/web/src/lib/worker-admin/browser-capability.ts Resolved

fix(admin): scope town auth by action target and bound caches

dfaff1d

kilo-code-bot Bot reviewed Nov 3, 2026

View reviewed changes

Comment thread apps/web/src/lib/worker-admin/browser-capability.test.ts Resolved

fix(gastown): bind user-target town routes to the named owner

4e07d97

pandemicsyn changed the base branch from feat/admin-assurance-inventory to feat/admin-mfa-step-up November 3, 2026 17:34

pandemicsyn added this pull request to stack #9714 November 3, 2026 17:37

pandemicsyn added 2 commits November 3, 2026 11:38

fix(gastown): bind user capabilities only after ordinary ownership

d544c53

fix(gastown): remove the pre-ownership user-target gate

3122785

kilo-code-bot Bot reviewed Nov 3, 2026

View reviewed changes

Comment thread services/gastown/src/middleware/town-auth.middleware.ts Resolved

test(admin): prove capability expiry re-mint and audit denied user ta…

0c10c41

…rgets

pandemicsyn mentioned this pull request Nov 3, 2026

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

Open

Merge remote-tracking branch 'origin/feat/admin-mfa-step-up' into fea…

dedbd2c

…t/admin-worker-capability # Conflicts: # packages/web-shared/src/lib/admin/privileged-inventory.ts

pandemicsyn mentioned this pull request Nov 3, 2026

feat(admin): add privileged access contract and inventory #9610

Open

Conversation

pandemicsyn commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes

Before merge / deploy

To close the gap later (config only, needs #9615 live)

Verification

Visual Changes

Uh oh!

Uh oh!

Uh oh!

kilo-code-bot Bot commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Executive Summary

Previous review (commit 3122785)

Executive Summary

Overview

SUGGESTION

Previous review (commit dfaff1d)

Executive Summary

Overview

SUGGESTION

Previous review (commit ec160d8)

Executive Summary

Overview

WARNING

SUGGESTION

Previous review (commit 0db16a6)

Executive Summary

Overview

WARNING

SUGGESTION

Previous review (commit 22f1d21)

Executive Summary

Overview

WARNING

SUGGESTION

Previous review (commit 58f522a)

Executive Summary

Overview

WARNING

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

pandemicsyn commented Nov 2, 2026 •

edited

Loading

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

edited

Loading

Previous review (commit `3122785`)

Previous review (commit `dfaff1d`)

Previous review (commit `ec160d8`)

Previous review (commit `0db16a6`)

Previous review (commit `22f1d21`)

Previous review (commit `58f522a`)