Skip to content

feat(kilo-mcp): gate admin and debug access behind an OTP opt-in - #6244

Merged
iscekic merged 4 commits into
mainfrom
kwf/owner-kilo-mcp-admin-checkbox-9ffb
Sep 18, 2026
Merged

iscekic merged 4 commits into
mainfrom
kwf/owner-kilo-mcp-admin-checkbox-9ffb

Conversation

@iscekic

@iscekic iscekic commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

Changelog for users

  • Admins see an off-by-default Enable admin and debug actions checkbox at sign-in; non-admins never see it or any admin or debug trace.
  • Ticking the box before enrolment reveals the one-time secret, its otpauth:// URI and a code field, without a page load.
  • A valid current code enables admin and debug actions; an empty or wrong code re-renders the picker and mints no grant.
  • An enrolled admin sees only the ticked checkbox, which stays ticked across reloads; ticking or unticking it toggles the actions.
  • When the admin check cannot be reached, the picker keeps the organisation list and Connect and shows the reload notice.
  • Guarded endpoints stay hidden from search and call unless the connection opted in; only an admin is told they were hidden.
  • Opted-in admins get call_protected and submit_otp; all other connections see only search and call.
  • Codes are single use, wrong codes are rate limited, pending requests expire in five minutes, and only the creating connection may submit.

Changelog for maintainers

  • user.getMe now returns isAdmin, and the sign-in picker re-derives admin eligibility per request, so an edited form cannot grant admin or debug access.
  • Grants carry adminEnabled, adminEligible and a per-connection sessionId; each fails closed when absent, so pre-feature grants stay disabled.
  • Catalog rows gain admin and debug markers emitted only when true; a row is guarded when either is present, and the dump stays byte-stable.
  • The store adds mcp_admin_authenticators and mcp_protected_requests (migration 0006) and drops the approval-queue tables (migration 0007).
  • call and submit_otp share one upstream path; call_protected fixes the payload and binds it to sessionId, submit_otp re-derives the live admin role before claiming, and TOTP is a dependency-free RFC 6238 implementation over WebCrypto with no new runtime dependency.
  • services/kilo-mcp/src/store/oauth-store.ts:521 — accepted: MAX_OTP_ATTEMPTS only invalidates the current request and call_protected mints a fresh row with attempts = 0, so the count now lives on the authenticator (failed_attempts, locked_until, migration 0008); 10 wrong codes lock the account for 15 minutes and both peek and claim consult the lock.
  • Removed: the approval-queue app, its store methods and its form schema; every non-/authorize worker path 404s again.
  • Review the OTP claim transaction and the guarded-search gate first; the dev config marks Workers AI and Vectorize remote to exercise semantic search locally.

E2E proof

The earlier live-check summary is superseded by the log excerpts below, collected 2026-09-18. The consent/picker screenshots remain valid, and the change has no device surface, so the Android emulator was not exercised.

[e1] Admin check down on submit — picker enrolment

[e1] Admin check down on submit — picker healthy

[e1] Admin check down on submit — mobile picker enrolment

[e1] Admin check down on submit — mobile picker healthy

[e1] Admin check down on submit — stale submit notice

E2E proof — log excerpts

[e1] Admin check down on submit -> pass :: Platform: web/MCP (apps/web :6300 -> fault proxy :8899 -> kilo-mcp worker :8815, the setup PR #6244 names; the android emulator renders no screen for this scenario). A fresh live run this round ends '35/35 checks passed' in drive-e1-http.log, with e7-result.json holding 35 '"ok": true' and no false, including 'radios=1' (the picker keeps the org radios and Connect with the fault on), 'count=0 baseline=0' (the faulty admin check published no admin access and made no upstream admin call) and 'count=1' (the recovered submit_otp made exactly one upstream admin request); fault-proxy-e1.log carries six 'FAULT 500 GET /api/trpc/user.getMe' lines with 'PROXY 200 GET /api/trpc/organizations.list' sti

Comment thread services/kilo-mcp/src/oauth/pages.ts
Comment thread services/kilo-mcp/src/index.ts
Comment thread services/kilo-mcp/src/store/oauth-store.ts
@kilo-code-bot

kilo-code-bot Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • services/kilo-mcp/src/index.test.ts
Previous Review Summaries (4 snapshots, latest commit 57eacca)

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

Previous review (commit 57eacca)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (11 files)
  • services/kilo-mcp/drizzle/0008_watery_jazinda.sql
  • services/kilo-mcp/drizzle/meta/0008_snapshot.json
  • services/kilo-mcp/drizzle/meta/_journal.json
  • services/kilo-mcp/drizzle/migrations.js
  • services/kilo-mcp/src/db/sqlite-schema.ts
  • services/kilo-mcp/src/index.test.ts
  • services/kilo-mcp/src/index.ts
  • services/kilo-mcp/src/store/oauth-store.test.ts
  • services/kilo-mcp/src/store/oauth-store.ts
  • services/kilo-mcp/src/store/protected-requests.test.ts
  • services/kilo-mcp/src/types.ts

Previous review (commit 79bda6d)

Status: 1 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
services/kilo-mcp/src/store/oauth-store.ts 521 The wrong-code attempt cap is per protected request and call_protected can mint requests without bound, so OTP guessing has no global rate limit per authenticator.
Files Reviewed (40 files)
  • apps/web/src/routers/user-router.test.ts
  • apps/web/src/routers/user-router.ts
  • apps/web/src/scripts/mcp-catalog/catalog.test.ts
  • apps/web/src/scripts/mcp-catalog/catalog.ts
  • services/ai-attribution/drizzle/meta/_journal.json
  • services/kilo-mcp/catalog.json
  • services/kilo-mcp/drizzle/0005_approval_queue.sql
  • services/kilo-mcp/drizzle/0006_lovely_hammerhead.sql
  • services/kilo-mcp/drizzle/0007_drop_approval_queue.sql
  • services/kilo-mcp/drizzle/meta/0005_snapshot.json
  • services/kilo-mcp/drizzle/meta/0006_snapshot.json
  • services/kilo-mcp/drizzle/meta/0007_snapshot.json
  • services/kilo-mcp/drizzle/meta/_journal.json
  • services/kilo-mcp/drizzle/migrations.js
  • services/kilo-mcp/src/auth.test.ts
  • services/kilo-mcp/src/auth.ts
  • services/kilo-mcp/src/auth/http.ts
  • services/kilo-mcp/src/call.test.ts
  • services/kilo-mcp/src/call.ts
  • services/kilo-mcp/src/db/sqlite-schema.ts
  • services/kilo-mcp/src/deployment-config.test.ts
  • services/kilo-mcp/src/index.test.ts
  • services/kilo-mcp/src/index.ts
  • services/kilo-mcp/src/oauth/consent.test.ts
  • services/kilo-mcp/src/oauth/consent.ts
  • services/kilo-mcp/src/oauth/kilo-pairing.test.ts
  • services/kilo-mcp/src/oauth/kilo-pairing.ts
  • services/kilo-mcp/src/oauth/pages.test.ts
  • services/kilo-mcp/src/oauth/pages.ts
  • services/kilo-mcp/src/otp/totp.test.ts
  • services/kilo-mcp/src/otp/totp.ts
  • services/kilo-mcp/src/schemas.test.ts
  • services/kilo-mcp/src/schemas.ts
  • services/kilo-mcp/src/search.test.ts
  • services/kilo-mcp/src/search.ts
  • services/kilo-mcp/src/store/oauth-store.test.ts
  • services/kilo-mcp/src/store/oauth-store.ts
  • services/kilo-mcp/src/store/protected-requests.test.ts
  • services/kilo-mcp/src/types.ts
  • services/kilo-mcp/wrangler.jsonc

Fix these issues in Kilo Cloud

Previous review (commit c19c388)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
services/kilo-mcp/src/oauth/pages.ts 108 The new authenticator enrolment subsection renders the long-lived TOTP secret in a response that has no Cache-Control: no-store (htmlResponse sets only Content-Type), so browser/intermediary caches may retain it.

SUGGESTION

File Line Issue
services/kilo-mcp/src/index.ts 493 case 'expired' and case 'invalidated' are unreachable: peekProtectedRequest collapses those states to gone, so their refusal copy is never produced in production (the outcome test uses a fake store).
services/kilo-mcp/src/store/oauth-store.ts 449 verifyOtpAndClaim does not require verified_at, contradicting the schema's documented an unverified row is unusable invariant; not currently reachable but fails open.
Files Reviewed (39 files)
  • services/kilo-mcp/src/otp/totp.ts (new) - no issues
  • services/kilo-mcp/src/index.ts - 1 issue
  • services/kilo-mcp/src/call.ts - no issues
  • services/kilo-mcp/src/search.ts - no issues
  • services/kilo-mcp/src/schemas.ts - no issues
  • services/kilo-mcp/src/types.ts - no issues
  • services/kilo-mcp/src/auth.ts - no issues
  • services/kilo-mcp/src/oauth/consent.ts - no issues
  • services/kilo-mcp/src/oauth/pages.ts - 1 issue
  • services/kilo-mcp/src/oauth/kilo-pairing.ts - no issues
  • services/kilo-mcp/src/store/oauth-store.ts - 1 issue
  • services/kilo-mcp/src/db/sqlite-schema.ts - no issues
  • apps/web/src/routers/user-router.ts - no issues
  • apps/web/src/scripts/mcp-catalog/catalog.ts - no issues
  • services/kilo-mcp/catalog.json - no issues
  • services/kilo-mcp/wrangler.jsonc - no issues
  • services/kilo-mcp/drizzle/0005_approval_queue.sql (new) - no issues
  • services/kilo-mcp/drizzle/0006_lovely_hammerhead.sql (new) - no issues
  • services/kilo-mcp/drizzle/0007_drop_approval_queue.sql (new) - no issues
  • services/kilo-mcp/drizzle/migrations.js - no issues
  • services/kilo-mcp/drizzle/meta/_journal.json - no issues
  • Tests (index, call, search, schemas, consent, pages, kilo-pairing, auth, totp, oauth-store, protected-requests, catalog, user-router, deployment-config) - no issues

Fix these issues in Kilo Cloud

Previous review (commit eed4b3a)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
services/kilo-mcp/src/oauth/pages.ts 108 The new authenticator enrolment subsection renders the long-lived TOTP secret in a response that has no Cache-Control: no-store (htmlResponse sets only Content-Type), so browser/intermediary caches may retain it.

SUGGESTION

File Line Issue
services/kilo-mcp/src/index.ts 493 case 'expired' and case 'invalidated' are unreachable: peekProtectedRequest collapses those states to gone, so their refusal copy is never produced in production (the outcome test uses a fake store).
services/kilo-mcp/src/store/oauth-store.ts 449 verifyOtpAndClaim does not require verified_at, contradicting the schema's documented an unverified row is unusable invariant; not currently reachable but fails open.
Files Reviewed (39 files)
  • services/kilo-mcp/src/otp/totp.ts (new) - no issues
  • services/kilo-mcp/src/index.ts - 1 issue
  • services/kilo-mcp/src/call.ts - no issues
  • services/kilo-mcp/src/search.ts - no issues
  • services/kilo-mcp/src/schemas.ts - no issues
  • services/kilo-mcp/src/types.ts - no issues
  • services/kilo-mcp/src/auth.ts - no issues
  • services/kilo-mcp/src/oauth/consent.ts - no issues
  • services/kilo-mcp/src/oauth/pages.ts - 1 issue
  • services/kilo-mcp/src/oauth/kilo-pairing.ts - no issues
  • services/kilo-mcp/src/store/oauth-store.ts - 1 issue
  • services/kilo-mcp/src/db/sqlite-schema.ts - no issues
  • apps/web/src/routers/user-router.ts - no issues
  • apps/web/src/scripts/mcp-catalog/catalog.ts - no issues
  • services/kilo-mcp/catalog.json - no issues
  • services/kilo-mcp/wrangler.jsonc - no issues
  • services/kilo-mcp/drizzle/0005_approval_queue.sql (new) - no issues
  • services/kilo-mcp/drizzle/0006_lovely_hammerhead.sql (new) - no issues
  • services/kilo-mcp/drizzle/0007_drop_approval_queue.sql (new) - no issues
  • services/kilo-mcp/drizzle/migrations.js - no issues
  • services/kilo-mcp/drizzle/meta/_journal.json - no issues
  • Tests (index, call, search, schemas, consent, pages, kilo-pairing, auth, totp, oauth-store, protected-requests, catalog, user-router, deployment-config) - no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic marked this pull request as draft September 16, 2026 21:08
@iscekic
iscekic force-pushed the kwf/owner-kilo-mcp-admin-checkbox-9ffb branch from 27669e8 to c19c388 Compare September 16, 2026 22:09
@iscekic
iscekic marked this pull request as ready for review September 16, 2026 22:24
@iscekic
iscekic marked this pull request as draft September 16, 2026 22:28
@iscekic
iscekic marked this pull request as ready for review September 16, 2026 22:57
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 17, 2026
@iscekic iscekic self-assigned this Sep 17, 2026
@iscekic
iscekic force-pushed the kwf/owner-kilo-mcp-admin-checkbox-9ffb branch from c19c388 to 79bda6d Compare September 18, 2026 02:40
Comment thread services/kilo-mcp/src/store/oauth-store.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants