feat(auth): make revocation and account deletion real (w4c) - #5337
Merged
Conversation
…loud-agent JWT auth
P2-D-24b: getOrganizationGroupPolicyContext without tx opened readDb. The openrouter gateway admission path consumes that membership read. Read the primary (db) instead, keeping repeatable-read read-only.
P2-D-23a: bearer writes and sign-out deletes used no keychain class. Use SecureStore.WHEN_UNLOCKED_THIS_DEVICE_ONLY so the tokens are not in iCloud or iTunes backup and do not migrate to a new device.
…lient
P2-GH-54c: mobile and extension getTicket parsed only { ticket } and returned
a string, so the SDK set expiresAt to undefined. Return { ticket, expiresAt }
and fail when expiresAt is missing. CodeReviewStreamView keeps a string
handoff to websocket-manager.
… on removal P2-D-22: handleWebCommand rechecks current org membership before forwarding. Member removal closes the member's live UserConnection web sockets and Cloud Agent stream sockets via closeViewerSockets and closeOrgStreams.
P1-D-34a/b: requestAccountDeletionChallenge emails a sign-in code and returns devCode in non-production. requestAccountDeletion reauthenticates with the code, checks preconditions, then calls performGdprRemoval. Mobile keeps the challenge on precondition and invalid-code errors so the user can resubmit.
Fix tsgo errors (body cast, NODE_ENV replaceProperty, nonce-DO binding, holder pattern, requireActual cast, fetch mock typing) and oxlint no-floating-promises / require-await in the slice test files.
The extension getTicket now requires expiresAt; the e2e fixture mock omitted it and would fail Cloud Agent connect.
P3-D-08ba requires stream and terminal replay rejection; /stream had a test but /terminal did not.
Creates an organization with an owner and a member for the S4 socket-close scenario.
iscekic
changed the title
…9496 # Conflicts: # services/session-ingest/src/index.test.ts # services/session-ingest/worker-configuration.d.ts
connectionTicketConsumeResponseSchema is only used via z.infer to derive a type, never safeParse'd. Replace it with a plain type.
validateKiloToken imports verifyKiloBearerAgainstCurrentPepper, which transitively imports pg. auth.ts is imported by CloudAgentSession, which the integration-test worker imports, so pg leaked into the Workers runtime and failed to load. Move validateKiloToken into its own module so auth.ts stays pg-free.
…9496 # Conflicts: # apps/web/src/routers/active-sessions-router.test.ts # apps/web/src/routers/active-sessions-router.ts # packages/worker-utils/src/kilo-token-auth.ts # services/session-ingest/src/middleware/kilo-jwt-auth.test.ts # services/session-ingest/src/middleware/kilo-jwt-auth.ts
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental change is a test-only branded-type cast in the mobile session-manager fixture; prior revocation and deletion findings remain resolved. Files Reviewed (1 file)
Previous Review Summaries (4 snapshots, latest commit 151820c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 151820c)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental fixes consume the deletion code only after GDPR removal and map auth dependency outages to 503; both prior findings are resolved. Files Reviewed (6 files)
Previous review (commit 54c4c1f)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryAccount deletion still burns the confirmation code before Overview
Issue Details (click to expand)WARNING
Files Reviewed (71 files)
Fix these issues in Kilo Cloud Previous review (commit 412d878)Status: 5 Issues Found | Recommendation: Address before merge Executive SummaryAccount deletion reauth skips expiry and attempt limits, and one-use ingest tickets are replayed on unexpected reconnect. Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (70 files)
Fix these issues in Kilo Cloud Previous review (commit ac56293)Status: 5 Issues Found | Recommendation: Address before merge Executive SummaryAccount deletion reauth skips expiry and attempt limits, and one-use ingest tickets are replayed on unexpected reconnect. Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (70 files)
Reviewed by grok-4.6 · Input: 116.3K · Output: 5.1K · Cached: 304.4K Review guidance: REVIEW.md from base branch |
Reserve the sign-in code before consuming it so an expired (>10 min) code and the 5-attempt budget are enforced, matching the normal sign-in flow. Treat production provider_not_configured as a send failure instead of stamping a cooldown and returning a codeless success.
The ingest ticket is consumed once at the /api/user/web upgrade. Refresh it before a reconnect so a non-auth-failure close (1006) does not retry with an already-consumed ticket and loop forever. hasEverOpened scopes the refresh to reconnects so the initial connect does not double-mint.
Set an alarm on nonce consume and delete the used flag in alarm() so nonce DOs do not accumulate storage forever. The flag only needs to persist until the ticket JWT expires, after which validateStreamTicket rejects the replay.
Contributor
Author
|
(bot) Rejected the remaining WARNING (user-router.ts:978 — code consumed before |
pandemicsyn
approved these changes
Aug 18, 2026
A failed performGdprRemoval burned the confirmation code, and the 1 h request cooldown then blocked a new one. Release the reservation on failure and consume the code after removal succeeds, so the client retry with the same challenge works.
verifyKiloBearerAgainstCurrentPepper swallowed secret-store and database failures into null, so Session Ingest answered an outage with 'Invalid or expired token'. Let dependency errors propagate and map them to a retryable 503; null now means an invalid or revoked credential only.
iscekic
enabled auto-merge (squash)
August 18, 2026 18:38
iscekic
added a commit
that referenced
this pull request
Aug 18, 2026
PR #5337 gave the mutation a required input, so builds already in the stores get a 400 on the delete-account flow. Make the input optional and keep the old support-ticket path for a call without one: it emails the user and support, stamps the cooldown, and deletes nothing, which is exactly what those builds tell the user happened. A call with a challenge still reauthenticates and performs the GDPR removal. The branch carries a TODO for its removal.
iscekic
added a commit
that referenced
this pull request
Aug 18, 2026
* fix(active-sessions): restore getToken as a query for shipped clients PR #5337 changed activeSessions.getToken from a query to a mutation. Every installed client calls it as a query, so tRPC rejects the call: 405 on a plain GET, and 400 "Cannot mix procedure types in call" on any batch that pairs it with user.getMe. The mobile app then shows "Could not load your account". Shipped app builds and installed extensions cannot be updated in step with the server, so the procedure type must stay a query. The one-use web-ticket body is unchanged. * feat(active-sessions): add createWebTicket mutation, keep getToken query Minting is not idempotent, so createWebTicket is a mutation and is the path forward. Web, mobile and extension now call it. getToken stays a query for clients that are already installed, with a TODO for its removal. Shipped app builds and installed extensions cannot update in step with the server, and tRPC answers a query-shaped call to a mutation with 405, or fails the whole batch with 400 "Cannot mix procedure types in call" when it sits beside a query. Both procedures share one mintWebTicket handler. * fix(user): accept requestAccountDeletion without a challenge again PR #5337 gave the mutation a required input, so builds already in the stores get a 400 on the delete-account flow. Make the input optional and keep the old support-ticket path for a call without one: it emails the user and support, stamps the cooldown, and deletes nothing, which is exactly what those builds tell the user happened. A call with a challenge still reauthenticates and performs the GDPR removal. The branch carries a TODO for its removal.
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
What changed for the user
How it works, for the maintainer
Kilo bearer verification now checks the account's current API-token pepper and blocked state. A rotated pepper or a blocked account fails even when a cached existence check would pass, so revocation takes effect immediately. A secret-store or database failure throws instead of returning null, so callers map an outage to a retryable 503 rather than a 401.
Files
packages/worker-utils/src/kilo-token-auth.ts— addsfindKiloUserPepperandverifyKiloBearerAgainstCurrentPepper.Session Ingest now verifies the current pepper and mints one-use web tickets. The
/api/user/webupgrade consumes an opaque ticket before any JWT path runs, and a dependency failure maps to 503. The ticket lives in a new Durable Object and is consumed exactly once.Files
services/session-ingest/src/middleware/kilo-jwt-auth.ts— verifies against the current pepper; adds the one-use web-ticket branch that runs before the JWT path.services/session-ingest/src/dos/connection-ticket-do.ts— new Durable Object:mintstores{ userId, expiresAt }and arms an alarm;consumedeletes atomically and returns the userId exactly once.services/session-ingest/src/routes/api.ts— addsPOST /api/user/web-ticket(opaque UUID, 60 s expiry);/api/user/webforwards the authenticatedkiloUserId.services/session-ingest/src/index.ts— exportsConnectionTicketDO.services/session-ingest/wrangler.jsonc— adds theCONNECTION_TICKET_DObinding and av3SQLite migration.services/session-ingest/wrangler.test.jsonc— adds theCONNECTION_TICKET_DObinding and av2test migration.Session Ingest rechecks membership on subscribe and command, and closes viewer sockets on member removal. A removed member gets a
SESSION_ACCESS_DENIEDenvelope and their live sockets close immediately. The invalidate endpoint now also closes viewer sockets.Files
services/session-ingest/src/dos/UserConnectionDO.ts— carrieskiloUserIdon the web attachment; rechecks membership viaresolveAccessibleKiloSessionon bothsubscribeandcommand; addscloseViewerSockets().services/session-ingest/src/app.ts—/internal/session-access/invalidatealso callscloseViewerSockets().Cloud Agent Next tRPC auth now enforces the current pepper and active-account state.
validateKiloTokenwraps the shared verifier and passes the Hyperdrive connection string. A blocked or pepper-rotated account fails tRPC auth.Files
services/cloud-agent-next/src/validate-kilo-token.ts— newvalidateKiloTokenwrapper around the shared verifier.services/cloud-agent-next/src/middleware/auth.ts— usesvalidateKiloTokeninstead of the previous token check.Cloud Agent stream and terminal tickets are now audience-scoped and single-use, and member removal closes live streams. The worker
/streamand/terminalroutes validate the audience, recheck session access, and consume a ticket nonce exactly once before upgrading, so a replayed or cross-purpose ticket is rejected; the direct Durable Object/streamvalidates audience only. Removing an organization member also closes every live stream socket for that member through a new internal endpoint.Files
services/cloud-agent-next/src/auth.ts— addsSTREAM_TICKET_AUDIENCEandTERMINAL_TICKET_AUDIENCE;validateStreamTicketrequires anexpectedAudience; the legacy wrapper path re-verifies throughverifyKiloToken.services/cloud-agent-next/src/persistence/StreamTicketNonceDO.ts— new one-time nonce consumer;consumeStreamTicketNonceis the single consume call site.services/cloud-agent-next/src/server.ts— validates audience and consumes the nonce on/streamand/terminal; addsPOST /internal/streams/close.services/cloud-agent-next/src/persistence/CloudAgentSession.ts— passesSTREAM_TICKET_AUDIENCEtovalidateStreamTicket; addscloseOrgStreams(organizationId).services/cloud-agent-next/src/types.ts— declares theSTREAM_TICKET_NONCE_DObinding.services/cloud-agent-next/src/index.ts— exportsStreamTicketNonceDO.services/cloud-agent-next/wrangler.jsonc— adds theSTREAM_TICKET_NONCE_DObinding and av8SQLite migration.services/cloud-agent-next/wrangler.test.jsonc— adds theSTREAM_TICKET_NONCE_DObinding and av8migration.The active-sessions
getTokenprocedure is now a mutation that mints a one-use web ticket. It posts to Session Ingest/api/user/web-ticketwith an internal service token and returns{ token, expiresAt }. A missing worker URL or a bad mint response fails fast.Files
apps/web/src/routers/active-sessions-router.ts—getTokenbecomes a mutation returning{ token, expiresAt }.Account deletion now requires an emailed confirmation code and performs real GDPR removal.
requestAccountDeletionChallengemints and emails a sign-in code with a 1-hour cooldown;requestAccountDeletionrechecks the soft-delete precondition, reserves the code, runs removal, and consumes the code only after removal succeeds. A failed removal releases the code so the caller is not stranded.Files
apps/web/src/routers/user-router.ts— addsrequestAccountDeletionChallengeandrequestAccountDeletion.The web app signs stream tickets with an audience.
signStreamTicketsets the JWT audience to stream or terminal based on purpose, and embeds a nonce.Files
apps/web/src/lib/cloud-agent/stream-ticket.ts— setsaudienceandnonceonsignStreamTicket.Member removal now closes the removed member's live Cloud Agent streams, and group-policy admission reads the primary database.
closeCloudAgentOrgStreamsposts to the internal close endpoint, and removal calls it best-effort. Group-policy admission reads the primary db instead of the replica when no transaction is passed.Files
apps/web/src/lib/cloud-agent-next/cloud-agent-client.ts— addscloseCloudAgentOrgStreams.apps/web/src/lib/organizations/organizations.ts— callscloseCloudAgentOrgStreamsafter member removal.apps/web/src/lib/organizations/organization-group-policy-context.server.ts— reads the primarydbinstead ofreadDb.The code-review stream view fetches a fresh stream ticket, and the Cloud Agent provider mints the ingest ticket as a mutation.
fetchStreamTicketvalidates{ ticket, expiresAt };CodeReviewStreamViewuses it.CloudAgentProviderswitchesgetTokento.mutate().Files
apps/web/src/components/code-reviews/fetch-stream-ticket.ts— newfetchStreamTicket.apps/web/src/components/code-reviews/CodeReviewStreamView.tsx— usesfetchStreamTicket.apps/web/src/components/cloud-agent-next/CloudAgentProvider.tsx—getTokencalls.mutate().iOS bearer tokens are stored with
WHEN_UNLOCKED_THIS_DEVICE_ONLY. This keeps credentials out of iCloud and iTunes backups so they never follow a device restore. Every bearer-token write and delete pins the option.Files
apps/mobile/src/lib/auth/credentials.ts— addsIOS_BEARER_SECURE_STORE_OPTIONSand pins every bearer write and delete to it.apps/mobile/src/lib/auth/auth-context.tsx— uses the same options on sign-out deletion.The mobile profile screen gains a Delete Account flow.
useDeleteAccountrequests a challenge, submits the code, and signs out on success. The profile screen renders the confirm, code-entry, and terminal states.Files
apps/mobile/src/components/use-delete-account.ts— new deletion hook.apps/mobile/src/components/profile-screen.tsx— adds the Delete Account flow.Mobile mints the ingest ticket as a mutation and returns
{ ticket, expiresAt }.mobile-session-managerreturns{ ticket, expiresAt }fromgetTicket;user-web-connection-providerswitchesgetTokento.mutate().Files
apps/mobile/src/components/agents/mobile-session-manager.ts—getTicketreturns{ ticket, expiresAt }.apps/mobile/src/components/agents/user-web-connection-provider.tsx—getTokencalls.mutate().The extension mints the ingest ticket as a mutation and returns
{ ticket, expiresAt }.extension-agent-session-managerreturns{ ticket, expiresAt }fromgetTicket;agents-providerswitchesgetTokento.mutate().Files
apps/extension/src/shared/extension-agent-session-manager.ts—getTicketreturns{ ticket, expiresAt }.apps/extension/entrypoints/sidepanel/agents-provider.tsx—getTokencalls.mutate().The SDK sends the one-use ticket under the
ticketquery parameter and refreshes it on reconnect.user-web-connectiontrackshasEverOpenedand refreshes auth only before a reconnect, so the initial connect does not double-mint.base-connectionadds askipAuthRefreshflag so the post-refresh reconnect does not refresh again.Files
packages/cloud-agent-sdk/src/user-web-connection.ts— sends the ticket under theticketparam; addshasEverOpenedandshouldRefreshAuthBeforeConnect.packages/cloud-agent-sdk/src/base-connection.ts— addsskipAuthRefreshtoconnectInternal.A seed fixture creates an owner-plus-member organization pair for the member-revocation E2E scenario.
Files
dev/seed/app/w4c-org-pair.ts— new seed.Tests: 30 test files changed (9 new), covering the shared pepper verification, the one-use web ticket, the nonce replay rejection, the member-removal socket close, and the mobile deletion flow.
Generated: 2
worker-configuration.d.tsfiles regenerated (session-ingest and cloud-agent-next).Verification
Visual Changes
The mobile Profile screen gains a Delete Account flow (confirm, enter emailed code, terminal success).
Reviewer Notes
/streamand/terminalpaths; the direct Durable Object/streamvalidates audience only (recorded decision — replay is guaranteed on the public worker path).activeSessions.getTokenis now a mutation because a one-use ticket must be minted on every connect.E2E: bot-e2e