feat(gastown): admin impersonation — allow admins to view any town#1469
Merged
feat(gastown): admin impersonation — allow admins to view any town#1469
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Reviewed by gpt-5.4-20260305 · 799,147 tokens |
Add admin impersonation support for the gastown worker so Kilo admins can navigate to any town URL for support, debugging, and monitoring. Key changes: - Add admin bypass to tRPC ownership verification (resolveTownOwnership, verifyTownOwnership, verifyRigOwnership) so admins can access any town - Block admins from destructive actions (deleteTown, deleteRig) and config modifications on towns they don't own - Mask secrets in town config for admin viewers (same as non-owner org members) - Add admin audit logging middleware that writes admin town access events to Cloudflare Analytics Engine - Add checkAdminAccess tRPC query so the frontend can detect admin viewing - Add AdminViewingBanner component shown on town overview and settings pages - Force read-only mode on settings page when admin is viewing another user's town - Allow admins to bypass the gastown-access feature flag - Add View Town UI button to admin panel (TownInspectorDashboard and UserAdminGastown) - Add unit tests for admin audit middleware and town-auth admin bypass Closes #1467
jrf0110
force-pushed
the
1467-admin-town-impersonation
branch
from
66a8d3a to
b7a9393
Compare
- Fix verifyRigOwnership for admins: add optional townIdHint parameter that resolves the real owner from TownDO config, so admin viewers can access rig-scoped queries (listBeads, listAgents, getRig, etc.) - Fix checkAdminAccess to return ownerOrgId for org-owned towns; update AdminViewingBanner to display org ownership context instead of null - Fix resolveTownOwnership catch block: no longer returns admin bypass for deleted/invalid towns — getTownConfig failure always throws NOT_FOUND - Block refreshContainerToken for admin viewers (same guard as updateTownConfig)
jrf0110
force-pushed
the
1467-admin-town-impersonation
branch
from
477fb65 to
9728410
Compare
| ) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const rig = await verifyRigOwnership(ctx.env, ctx.userId, input.rigId, ctx.orgMemberships); | ||
| const rig = await verifyRigOwnership(ctx.env, ctx, input.rigId, input.townId); |
Contributor
There was a problem hiding this comment.
WARNING: Admin viewers can still delete rig-scoped resources
verifyRigOwnership() now resolves another owner's rig whenever an admin supplies townId, so this mutation lets admin viewers delete beads in towns they do not own. The same authorization hole is still present in deleteAgent below, which means the impersonation flow is not actually restricting destructive actions to read-only support access.
Contributor
There was a problem hiding this comment.
i think thats the point
|
|
||
| // Admins see masked secrets (read-only access, no secret exposure) | ||
| if (ownership.type === 'admin') { | ||
| const mask = (s?: string) => (s ? '****' + s.slice(-4) : undefined); |
Contributor
There was a problem hiding this comment.
WARNING: Short secrets are still exposed by the new masking helper
mask() always appends the last four characters, so any secret or env var with length <= 4 is revealed in full ("abc" -> "****abc"). This PR is trying to guarantee that admin viewers never see another town's secrets, but short values still leak through both this helper and the env_vars masking below.
pandemicsyn
approved these changes
Mar 24, 2026
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
2 participants
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
resolveTownOwnership,verifyTownOwnership,verifyRigOwnership) so Kilo admins can access any town's routes — beads, agents, convoys, terminal, settings, mayor chat — without being the town owneradminAuditMiddleware) that writesadmin.town_accessevents to Cloudflare Analytics Engine with admin user ID, town ID, and routecheckAdminAccesstRPC query andAdminViewingBannercomponent that shows "Viewing as admin — this town belongs to user@..." when an admin views another user's towngastown-accessPostHog feature flagVerification
pnpm typecheck)pnpm testincloudflare-gastown/)pnpm lint)pnpm build)Visual Changes
Reviewer Notes
townAuthMiddlewareandtownOwnershipMiddlewarealready had admin bypass (if (c.get('kiloIsAdmin')) return next()). The gap was in the tRPC layer whereresolveTownOwnership/verifyTownOwnership/verifyRigOwnershipdid not checkisAdmin. This PR adds a new'admin'ownership type that propagates through the tRPC flow.{ type: 'admin' }fromresolveTownOwnership. Callers check for this type to block destructive mutations while allowing read access and mayor chat.checkAdminAccesstRPC query is cheap — it only hits the TownDO config when the user is an admin, and returns immediately for non-admins.writeEventutility. No new tables or migrations required.