Add Connect client approval mode#3582
Open
juliusmarminge wants to merge 7 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds a "client-approval" security mode to T3 Connect. When enabled, relay credential minting returns a ChangesConnect Client Approval Mode
Sequence Diagram(s)sequenceDiagram
participant Client as Client Runtime
participant Relay as Relay HTTP API
participant Connector as EnvironmentConnector
participant Server as Cloud Mint Handler
participant Store as ConnectClientStore
Client->>Relay: connectEnvironment(client, clientProofKeyThumbprint)
Relay->>Connector: connect(client, clientProofKeyThumbprint)
Connector->>Server: POST mintCredential(clientProofKeyThumbprint, client)
Server->>Store: authorizeConnectClientRequest(cloudUserId, thumbprint)
alt mode=client-approval AND status≠approved
Store-->>Server: {mode: "client-approval", status: "pending"}
Server-->>Connector: pending_approval proof JWT
Connector-->>Relay: {status: "pending_approval", approvalStatus, requestedAt}
Relay-->>Client: pending_approval response
Client-->>Client: ConnectionBlockedError(reason: "permission")
else approved or account mode
Store-->>Server: {mode: "account" or status: "approved"}
Server-->>Connector: authorized credential + expiresAt
Connector-->>Relay: {credential, expiresAt}
Relay-->>Client: authorized connect response
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Revoke race still mints credentials
- Changed the fallback when markSeen returns Option.none() from returning status 'approved' to 'rejected', so a client revoked/rejected between upsert and markSeen will not receive credentials.
- ✅ Fixed: Stale last seen after re-register
- Added last_seen_at to the upsert ON CONFLICT UPDATE SET clause with a CASE that resets it to NULL when revoked_at is not null (revoked client re-registering), matching the pattern used for approved_at and rejected_at.
Or push these changes by commenting:
@cursor push 87ff841023
Preview (87ff841023)
diff --git a/apps/server/src/auth/ConnectClientStore.ts b/apps/server/src/auth/ConnectClientStore.ts
--- a/apps/server/src/auth/ConnectClientStore.ts
+++ b/apps/server/src/auth/ConnectClientStore.ts
@@ -333,7 +333,7 @@
return { mode, status: "approved" as const, client: seenClient };
}
- return { mode, status: "approved" as const, client: visibleClient };
+ return { mode, status: "rejected" as const, client: visibleClient };
});
const updateDecision = <E extends ConnectClientApprovalError | ConnectClientRejectionError>(
diff --git a/apps/server/src/persistence/AuthConnectClients.ts b/apps/server/src/persistence/AuthConnectClients.ts
--- a/apps/server/src/persistence/AuthConnectClients.ts
+++ b/apps/server/src/persistence/AuthConnectClients.ts
@@ -256,6 +256,10 @@
WHEN auth_connect_clients.revoked_at IS NULL THEN auth_connect_clients.rejected_at
ELSE NULL
END,
+ last_seen_at = CASE
+ WHEN auth_connect_clients.revoked_at IS NULL THEN auth_connect_clients.last_seen_at
+ ELSE NULL
+ END,
revoked_at = NULL
RETURNING ${sql.unsafe(rowSelection)}
`,You can send follow-ups to the cloud agent here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
apps/server/src/persistence/Migrations/033_AuthConnectClientApprovals.ts (1)
12-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd DB constraints for enum-like columns.
The repository decoders only accept known
statusandclient_device_typeliterals, but the table allows arbitrary text. AddCHECKconstraints now while this migration is new.Proposed migration hardening
- status TEXT NOT NULL, + status TEXT NOT NULL CHECK (status IN ('pending', 'approved', 'rejected')), client_label TEXT, client_ip_address TEXT, client_user_agent TEXT, - client_device_type TEXT NOT NULL DEFAULT 'unknown', + client_device_type TEXT NOT NULL DEFAULT 'unknown' + CHECK (client_device_type IN ('desktop', 'mobile', 'tablet', 'bot', 'unknown')),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/server/src/persistence/Migrations/033_AuthConnectClientApprovals.ts` around lines 12 - 16, Add database CHECK constraints for the enum-like columns in the 033_AuthConnectClientApprovals migration. Update the table definition so `status` only accepts the same known literals handled by the repository decoders, and `client_device_type` only accepts the supported device-type literals (including the existing default). Make the constraints part of the migration near the `CREATE TABLE` definition so `AuthConnectClientApprovals` cannot store arbitrary text in these fields.apps/mobile/src/features/cloud/linkEnvironment.test.ts (1)
838-838: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the new approval-blocked branch in this suite.
The production flow now short-circuits on credential-less relay responses, but this test update only verifies the renamed thumbprint field. Adding pending/rejected approval cases here would lock down the new user-visible errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/mobile/src/features/cloud/linkEnvironment.test.ts` at line 838, The linkEnvironment test suite only covers the renamed clientProofKeyThumbprint field and misses the new approval-blocked path in the cloud relay flow. Update the relevant tests around linkEnvironment to add cases for pending and rejected approval responses, and assert the user-visible errors thrown when the credential-less relay short-circuits. Use the existing linkEnvironment and relay-response fixtures in this suite so the new branch stays locked down.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/server/src/auth/ConnectClientStore.ts`:
- Around line 225-227: The decodeSecurityMode helper currently defaults unknown
persisted values to "account", which can silently downgrade security mode;
update decodeSecurityMode to accept only the two valid literals and throw a
ConnectSecurityModeLoadError for anything else. Then in getSecurityMode, catch
that decode failure and surface ConnectSecurityModeLoadError so corrupted or
unexpected stored values fail closed instead of falling back.
In `@apps/server/src/persistence/AuthConnectClients.ts`:
- Around line 271-274: The status update logic in AuthConnectClients should
clear the opposite decision timestamp when a client changes state, so the audit
fields stay consistent. Update the update statement around the approved_at and
rejected_at CASE expressions so that setting status to approved also resets
rejected_at, and setting status to rejected also resets approved_at, using the
existing status/decidedAt flow in the same persistence method.
- Around line 236-260: The upsert in AuthConnectClients should also reset stale
activity metadata when a revoked client is re-requested. In the ON CONFLICT ...
DO UPDATE branch for client_proof_key_thumbprint, update the logic in the same
statement that handles revoked_at/status/approved_at/rejected_at so last_seen_at
is cleared (set to NULL) when auth_connect_clients.revoked_at is not NULL, while
preserving it only for non-revoked rows.
In `@apps/web/src/components/settings/ConnectionsSettings.tsx`:
- Around line 3073-3090: The T3 Connect approval UI is still hidden behind the
local-network reachability gate, so relay-only/local-only backends never get
access to pending approvals. Update the rendering path in ConnectionsSettings,
especially the renderAuthorizedClients usage and the
isLocalBackendRemotelyReachable branch, so client approval rows are shown
whenever client-approval mode is enabled rather than only when the backend is
locally reachable. Keep the existing handlers like handleConnectClientAction and
the approve/reject/revoke props wired up, but remove the reachability dependency
that suppresses the approval list.
- Around line 2319-2322: The desktopConnectSecurityMode fallback in
ConnectionsSettings should not default to "account" before a snapshot is loaded.
Make the value nullable (or otherwise unknown) until authAccessChanges.data
provides a snapshot, and keep the approval toggle disabled while the mode is
unresolved so an initial load error does not misrepresent an existing
client-approval policy. Update the useMemo logic and any toggle/disabled checks
that consume desktopConnectSecurityMode to handle the null/unknown state
explicitly.
- Around line 2550-2575: The connect-client mutation handler currently only
tracks the exact action key, so other actions can still run while one request is
in flight. Update the action flow in ConnectionsSettings.tsx around the async
approve/reject/revoke handler to use a shared “any pending connect-client
mutation” guard, and prevent starting a new action when one is already running.
Keep the existing approveServerConnectClient, rejectServerConnectClient, and
revokeServerConnectClient logic, but ensure the pending state is set and cleared
in a way that blocks all connect-client buttons until the active mutation
settles.
In `@infra/relay/src/environments/EnvironmentConnector.ts`:
- Around line 258-264: The pending-approval verification path in
EnvironmentConnector is not authenticating requestedAt, so a stale or tampered
timestamp can still pass and be forwarded. Update the pending-approval proof
check in EnvironmentConnector to include requestedAt in the signed/verified
fields alongside approvalStatus and clientProofKeyThumbprint, and adjust the
pending-proof helper/contract in EnvironmentConnector.test.ts so it always
builds and compares requestedAt for pending-approval responses.
In `@packages/contracts/src/relay.ts`:
- Around line 807-839: The pending-approval proof payload is missing the
requestedAt field, so the timestamp on
RelayEnvironmentMintPendingApprovalResponse is not covered by the signed proof.
Update RelayEnvironmentMintPendingApprovalResponseProofPayload in relay.ts to
include requestedAt alongside the existing pending-approval claims, and then
adjust the signer and verifier paths that handle
RelayEnvironmentMintPendingApprovalResponse /
RelayEnvironmentMintPendingApprovalResponseProofPayload to require
proof.requestedAt to match response.requestedAt.
---
Nitpick comments:
In `@apps/mobile/src/features/cloud/linkEnvironment.test.ts`:
- Line 838: The linkEnvironment test suite only covers the renamed
clientProofKeyThumbprint field and misses the new approval-blocked path in the
cloud relay flow. Update the relevant tests around linkEnvironment to add cases
for pending and rejected approval responses, and assert the user-visible errors
thrown when the credential-less relay short-circuits. Use the existing
linkEnvironment and relay-response fixtures in this suite so the new branch
stays locked down.
In `@apps/server/src/persistence/Migrations/033_AuthConnectClientApprovals.ts`:
- Around line 12-16: Add database CHECK constraints for the enum-like columns in
the 033_AuthConnectClientApprovals migration. Update the table definition so
`status` only accepts the same known literals handled by the repository
decoders, and `client_device_type` only accepts the supported device-type
literals (including the existing default). Make the constraints part of the
migration near the `CREATE TABLE` definition so `AuthConnectClientApprovals`
cannot store arbitrary text in these fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ab5f94d1-7fa2-4c92-8c74-0ce01629ba60
📒 Files selected for processing (28)
apps/mobile/src/features/cloud/linkEnvironment.test.tsapps/mobile/src/features/cloud/linkEnvironment.tsapps/server/src/auth/ConnectClientStore.tsapps/server/src/auth/EnvironmentAuth.tsapps/server/src/auth/http.tsapps/server/src/cli/connect.tsapps/server/src/cloud/http.tsapps/server/src/persistence/AuthConnectClients.tsapps/server/src/persistence/Errors.tsapps/server/src/persistence/Migrations.tsapps/server/src/persistence/Migrations/033_AuthConnectClientApprovals.tsapps/server/src/ws.tsapps/web/src/cloud/linkEnvironment.test.tsapps/web/src/components/settings/ConnectionsSettings.tsxapps/web/src/environments/primary/auth.tsapps/web/src/environments/primary/index.tsapps/web/test/environmentHttpTest.tsinfra/relay/src/environments/EnvironmentConnector.test.tsinfra/relay/src/environments/EnvironmentConnector.tsinfra/relay/src/http/Api.tspackages/client-runtime/src/connection/resolver.test.tspackages/client-runtime/src/connection/resolver.tspackages/client-runtime/src/relay/managedRelay.tspackages/client-runtime/src/state/auth.test.tspackages/client-runtime/src/state/auth.tspackages/contracts/src/auth.tspackages/contracts/src/environmentHttp.tspackages/contracts/src/relay.ts
Comment on lines
+225
to
+227
| function decodeSecurityMode(bytes: Uint8Array): AuthConnectSecurityMode { | ||
| const value = textDecoder.decode(bytes).trim(); | ||
| return value === "client-approval" ? "client-approval" : "account"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed on invalid persisted security mode.
Line 227 silently maps any unknown stored value to "account", which disables client approval if the secret is corrupted or unexpectedly encoded. Validate both allowed literals and surface ConnectSecurityModeLoadError for anything else instead of downgrading.
Suggested direction
function decodeSecurityMode(bytes: Uint8Array): AuthConnectSecurityMode {
const value = textDecoder.decode(bytes).trim();
- return value === "client-approval" ? "client-approval" : "account";
+ if (value === "account" || value === "client-approval") {
+ return value;
+ }
+ throw new Error(`Invalid Connect security mode: ${value}`);
}Then convert that decode failure into ConnectSecurityModeLoadError in getSecurityMode.
Also applies to: 257-264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/server/src/auth/ConnectClientStore.ts` around lines 225 - 227, The
decodeSecurityMode helper currently defaults unknown persisted values to
"account", which can silently downgrade security mode; update decodeSecurityMode
to accept only the two valid literals and throw a ConnectSecurityModeLoadError
for anything else. Then in getSecurityMode, catch that decode failure and
surface ConnectSecurityModeLoadError so corrupted or unexpected stored values
fail closed instead of falling back.
Comment on lines
+271
to
+274
| status = ${status}, | ||
| updated_at = ${decidedAt}, | ||
| approved_at = CASE WHEN ${status} = 'approved' THEN ${decidedAt} ELSE approved_at END, | ||
| rejected_at = CASE WHEN ${status} = 'rejected' THEN ${decidedAt} ELSE rejected_at END, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clear the opposite decision timestamp on status changes.
Approving a previously rejected client leaves rejected_at set, and rejecting a previously approved client leaves approved_at set. That creates contradictory audit state.
Proposed fix
- approved_at = CASE WHEN ${status} = 'approved' THEN ${decidedAt} ELSE approved_at END,
- rejected_at = CASE WHEN ${status} = 'rejected' THEN ${decidedAt} ELSE rejected_at END,
+ approved_at = CASE WHEN ${status} = 'approved' THEN ${decidedAt} ELSE NULL END,
+ rejected_at = CASE WHEN ${status} = 'rejected' THEN ${decidedAt} ELSE NULL END,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| status = ${status}, | |
| updated_at = ${decidedAt}, | |
| approved_at = CASE WHEN ${status} = 'approved' THEN ${decidedAt} ELSE approved_at END, | |
| rejected_at = CASE WHEN ${status} = 'rejected' THEN ${decidedAt} ELSE rejected_at END, | |
| status = ${status}, | |
| updated_at = ${decidedAt}, | |
| approved_at = CASE WHEN ${status} = 'approved' THEN ${decidedAt} ELSE NULL END, | |
| rejected_at = CASE WHEN ${status} = 'rejected' THEN ${decidedAt} ELSE NULL END, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/server/src/persistence/AuthConnectClients.ts` around lines 271 - 274,
The status update logic in AuthConnectClients should clear the opposite decision
timestamp when a client changes state, so the audit fields stay consistent.
Update the update statement around the approved_at and rejected_at CASE
expressions so that setting status to approved also resets rejected_at, and
setting status to rejected also resets approved_at, using the existing
status/decidedAt flow in the same persistence method.
Comment on lines
+2319
to
+2322
| const desktopConnectSecurityMode: AuthConnectSecurityMode = useMemo(() => { | ||
| const event = authAccessChanges.data; | ||
| return event?.type === "snapshot" ? event.payload.connectSecurityMode : "account"; | ||
| }, [authAccessChanges.data]); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don't render an unknown approval mode as account.
Line 2321 falls back to "account" before a snapshot exists. Because the switch is only disabled while loading/mutating, a first-load error shows the backend as account-wide and lets the user overwrite an existing client-approval policy without ever seeing it. Keep this value nullable until the snapshot arrives and leave the toggle disabled in that state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/settings/ConnectionsSettings.tsx` around lines 2319 -
2322, The desktopConnectSecurityMode fallback in ConnectionsSettings should not
default to "account" before a snapshot is loaded. Make the value nullable (or
otherwise unknown) until authAccessChanges.data provides a snapshot, and keep
the approval toggle disabled while the mode is unresolved so an initial load
error does not misrepresent an existing client-approval policy. Update the
useMemo logic and any toggle/disabled checks that consume
desktopConnectSecurityMode to handle the null/unknown state explicitly.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review This PR introduces a significant new authentication feature (Connect client approval mode) with database schema changes, new API endpoints, and security-sensitive authorization logic. Changes to auth/security packages and new features affecting how clients are authorized require human review. You can customize Macroscope's approvability policy. Learn more. |
Contributor
There was a problem hiding this comment.
Effect service conventions: one finding in apps/server/src/auth/ConnectClientStore.ts — a pure validation failure fabricates an Error purely to satisfy a required cause.
Posted via Macroscope — Effect Service Conventions
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
There are 4 total unresolved issues (including 1 from previous review).
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Approval toggle stuck on load failure
- Reverted decodeSecurityMode to fall back to 'account' for invalid persisted values instead of failing with ConnectSecurityModeLoadError, so the subscription and UI toggle remain functional even with corrupt secret data.
- ✅ Resolved by another fix: Mint replay lost on mode load
- The decodeSecurityMode fix for Bug 1 eliminates the ConnectSecurityModeLoadError failure path that caused authorizeConnectClientRequest to fail after replay guards were already consumed.
- ✅ Fixed: Concurrent connect client actions
- Added a synchronous useRef guard (pendingConnectClientActionRef) that is checked and set before the asynchronous React state update, preventing concurrent clicks from both passing the guard before re-render.
Or push these changes by commenting:
@cursor push 0362fde9e2
Preview (0362fde9e2)
diff --git a/apps/server/src/auth/ConnectClientStore.ts b/apps/server/src/auth/ConnectClientStore.ts
--- a/apps/server/src/auth/ConnectClientStore.ts
+++ b/apps/server/src/auth/ConnectClientStore.ts
@@ -222,18 +222,9 @@
};
}
-function decodeSecurityMode(
- bytes: Uint8Array,
-): Effect.Effect<AuthConnectSecurityMode, ConnectSecurityModeLoadError> {
+function decodeSecurityMode(bytes: Uint8Array): AuthConnectSecurityMode {
const value = textDecoder.decode(bytes).trim();
- if (value === "account" || value === "client-approval") {
- return Effect.succeed(value);
- }
- return Effect.fail(
- new ConnectSecurityModeLoadError({
- cause: new Error(`Invalid Connect security mode: ${value}`),
- }),
- );
+ return value === "account" || value === "client-approval" ? value : "account";
}
function encodeSecurityMode(mode: AuthConnectSecurityMode): Uint8Array {
@@ -265,10 +256,10 @@
const getSecurityMode: ConnectClientStore["Service"]["getSecurityMode"] = () =>
secrets.get(CONNECT_SECURITY_MODE_SECRET).pipe(
+ Effect.map((mode) =>
+ Option.isSome(mode) ? decodeSecurityMode(mode.value) : ("account" as const),
+ ),
Effect.mapError((cause) => new ConnectSecurityModeLoadError({ cause })),
- Effect.flatMap((mode) =>
- Option.isSome(mode) ? decodeSecurityMode(mode.value) : Effect.succeed("account" as const),
- ),
Effect.withSpan("ConnectClientStore.getSecurityMode"),
);
diff --git a/apps/web/src/components/settings/ConnectionsSettings.tsx b/apps/web/src/components/settings/ConnectionsSettings.tsx
--- a/apps/web/src/components/settings/ConnectionsSettings.tsx
+++ b/apps/web/src/components/settings/ConnectionsSettings.tsx
@@ -8,7 +8,7 @@
TriangleAlertIcon,
} from "lucide-react";
import { useAuth } from "@clerk/react";
-import { type ReactNode, memo, useCallback, useEffect, useMemo, useState } from "react";
+import { type ReactNode, memo, useCallback, useEffect, useMemo, useRef, useState } from "react";
import {
AuthAccessReadScope,
AuthAccessWriteScope,
@@ -2224,6 +2224,7 @@
const [pendingConnectClientActionKey, setPendingConnectClientActionKey] = useState<string | null>(
null,
);
+ const pendingConnectClientActionRef = useRef(false);
const [addBackendDialogOpen, setAddBackendDialogOpen] = useState(false);
const [savedBackendMode, setSavedBackendMode] = useState<"remote" | "ssh">("remote");
const [savedBackendHost, setSavedBackendHost] = useState("");
@@ -2549,7 +2550,8 @@
const handleConnectClientAction = useCallback(
async (action: "approve" | "reject" | "revoke", clientProofKeyThumbprint: string) => {
- if (pendingConnectClientActionKey !== null) return;
+ if (pendingConnectClientActionRef.current) return;
+ pendingConnectClientActionRef.current = true;
const actionKey = `${action}:${clientProofKeyThumbprint}`;
setPendingConnectClientActionKey(actionKey);
setDesktopAccessManagementMutationError(null);
@@ -2573,10 +2575,11 @@
}),
);
} finally {
+ pendingConnectClientActionRef.current = false;
setPendingConnectClientActionKey(null);
}
},
- [pendingConnectClientActionKey],
+ [],
);
const handleAddSavedBackend = useCallback(async () => {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 136150d. Configure here.
Contributor
There was a problem hiding this comment.
One Effect service convention issue found. See inline comment.
Posted via Macroscope — Effect Service Conventions
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
1 participant
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
Verification
vp checkvp run typecheckvp run lint:mobilevp test packages/client-runtime/src/state/auth.test.ts packages/client-runtime/src/connection/resolver.test.ts infra/relay/src/environments/EnvironmentConnector.test.ts apps/mobile/src/features/cloud/linkEnvironment.test.ts apps/web/src/cloud/linkEnvironment.test.tsNotes
vp testwas run; unrelated provider tests failed in the full concurrent run but passed when rerun in isolation.Note
High Risk
Changes authentication and credential minting for cloud connect paths;
AuthAccessSnapshotand relay connect responses gain required/new fields that can break older clients if not upgraded together.Overview
Adds T3 Connect client approval: environments can use account-wide access or require per-client approval before relay clients receive credentials.
Server & persistence: New
ConnectClientStore, SQLiteauth_connect_clientstable, and HTTP APIs to read/set security mode and approve/reject/revoke clients byclientProofKeyThumbprint. Mint-credential in client-approval mode returns a signedpending_approvalJWT response instead of a pairing credential until the client is approved. Auth access WebSocket snapshots and live events now includeconnectSecurityModeand connect clients.Relay & clients: Connect requests carry optional client presentation metadata; connect/mint responses are a union of authorized credentials vs pending approval. Mobile, web, and
client-runtimeblock or surface clear errors when approval is pending or rejected.Product surfaces: Web Connections settings (toggle + client rows),
t3 connect securityCLI subcommands, and cloud link state exposingconnectSecurityMode. Renames connect payload field toclientProofKeyThumbprint(from deprecatedclientKeyThumbprint).Reviewed by Cursor Bugbot for commit 00ead3f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Connect client approval mode with approval/rejection/revocation workflows
client-approvalsecurity mode for T3 Connect, allowing server administrators to approve, reject, or revoke individual connecting clients before credentials are issued.ConnectClientStoreservice andAuthConnectClientspersistence layer (migration 033) to track client state (pending/approved/rejected) keyed by proof key thumbprint.pending_approvalresponse (instead of a credential) when mode isclient-approvaland the client is not yet approved; clients verify and surface this response as aConnectionBlockedError./api/auth/connect-*for managing mode and clients, and extends the WebSocket auth-access stream withconnectSecurityModeUpdated,connectClientUpserted, andconnectClientRemovedevents.t3 connect securityCLI subcommand to read/write security mode and list/approve/reject/revoke clients.ConnectClientListRowcomponent with per-client approve/reject/revoke actions.AuthAccessSnapshotnow requiresconnectSecurityModeandconnectClients; existing decoders will reject snapshots missing these fields.Macroscope summarized 00ead3f.
Summary by CodeRabbit
New Features
Bug Fixes