Implement server auth bootstrap and pairing flow#1768
Implement server auth bootstrap and pairing flow#1768juliusmarminge wants to merge 11 commits intot3code/remote-host-modelfrom
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Pairing URL points to root, token lost during redirect
- Added
url.pathname = "/pair"inissueStartupPairingUrlso the generated URL navigates directly to/pair?token=..., avoiding the redirect that strips the query parameter.
- Added
- ✅ Fixed: Secret store
setswallows write errors silently- Changed
Effect.map(() => new SecretStoreError(...))toEffect.flatMap(() => Effect.fail(new SecretStoreError(...)))so write failures properly propagate as Effect errors instead of being swallowed as success values.
- Changed
- ✅ Fixed: Bootstrap credential consume has TOCTOU race condition
- Replaced the non-atomic
Ref.get+Ref.updatesequence with a singleRef.modifycall that atomically reads the grant, validates it, and updates the map in one operation.
- Replaced the non-atomic
- ✅ Fixed: Duplicate one-time tokens issued during startup
- In non-desktop mode,
resolveStartupBrowserTargetis now called once and the resulting URL is reused for both logging and browser opening, avoiding issuing two separate one-time tokens.
- In non-desktop mode,
Or push these changes by commenting:
@cursor push f99e419f7c
Preview (f99e419f7c)
diff --git a/apps/server/src/auth/Layers/BootstrapCredentialService.ts b/apps/server/src/auth/Layers/BootstrapCredentialService.ts
--- a/apps/server/src/auth/Layers/BootstrapCredentialService.ts
+++ b/apps/server/src/auth/Layers/BootstrapCredentialService.ts
@@ -49,47 +49,60 @@
return credential;
});
+ type ConsumeResult =
+ | { readonly _tag: "error"; readonly error: BootstrapCredentialError }
+ | { readonly _tag: "ok"; readonly grant: BootstrapGrant };
+
const consume: BootstrapCredentialServiceShape["consume"] = (credential) =>
Effect.gen(function* () {
- const current = yield* Ref.get(grantsRef);
- const grant = current.get(credential);
- if (!grant) {
- return yield* new BootstrapCredentialError({
- message: "Unknown bootstrap credential.",
- });
- }
+ const now = yield* DateTime.now;
+ const result = yield* Ref.modify(
+ grantsRef,
+ (current): readonly [ConsumeResult, Map<string, StoredBootstrapGrant>] => {
+ const grant = current.get(credential);
+ if (!grant) {
+ return [
+ {
+ _tag: "error",
+ error: new BootstrapCredentialError({ message: "Unknown bootstrap credential." }),
+ },
+ current,
+ ];
+ }
- if (DateTime.isGreaterThanOrEqualTo(yield* DateTime.now, grant.expiresAt)) {
- yield* Ref.update(grantsRef, (state) => {
- const next = new Map(state);
- next.delete(credential);
- return next;
- });
- return yield* new BootstrapCredentialError({
- message: "Bootstrap credential expired.",
- });
- }
-
- const remainingUses = grant.remainingUses;
- if (typeof remainingUses === "number") {
- yield* Ref.update(grantsRef, (state) => {
- const next = new Map(state);
- if (remainingUses <= 1) {
+ if (DateTime.isGreaterThanOrEqualTo(now, grant.expiresAt)) {
+ const next = new Map(current);
next.delete(credential);
- } else {
- next.set(credential, {
- ...grant,
- remainingUses: remainingUses - 1,
- });
+ return [
+ {
+ _tag: "error",
+ error: new BootstrapCredentialError({ message: "Bootstrap credential expired." }),
+ },
+ next,
+ ];
}
- return next;
- });
+
+ const next = new Map(current);
+ const remainingUses = grant.remainingUses;
+ if (typeof remainingUses === "number") {
+ if (remainingUses <= 1) {
+ next.delete(credential);
+ } else {
+ next.set(credential, { ...grant, remainingUses: remainingUses - 1 });
+ }
+ }
+
+ return [
+ { _tag: "ok", grant: { method: grant.method, expiresAt: grant.expiresAt } },
+ next,
+ ];
+ },
+ );
+
+ if (result._tag === "error") {
+ return yield* result.error;
}
-
- return {
- method: grant.method,
- expiresAt: grant.expiresAt,
- } satisfies BootstrapGrant;
+ return result.grant;
});
return {
diff --git a/apps/server/src/auth/Layers/ServerAuth.ts b/apps/server/src/auth/Layers/ServerAuth.ts
--- a/apps/server/src/auth/Layers/ServerAuth.ts
+++ b/apps/server/src/auth/Layers/ServerAuth.ts
@@ -124,6 +124,7 @@
bootstrapCredentials.issueOneTimeToken().pipe(
Effect.map((credential) => {
const url = new URL(baseUrl);
+ url.pathname = "/pair";
url.searchParams.set("token", credential);
return url.toString();
}),
diff --git a/apps/server/src/auth/Layers/ServerSecretStore.ts b/apps/server/src/auth/Layers/ServerSecretStore.ts
--- a/apps/server/src/auth/Layers/ServerSecretStore.ts
+++ b/apps/server/src/auth/Layers/ServerSecretStore.ts
@@ -47,8 +47,10 @@
Effect.catch((cause) =>
fileSystem.remove(tempPath).pipe(
Effect.orElseSucceed(() => undefined),
- Effect.map(
- () => new SecretStoreError({ message: `Failed to persist secret ${name}.`, cause }),
+ Effect.flatMap(() =>
+ Effect.fail(
+ new SecretStoreError({ message: `Failed to persist secret ${name}.`, cause }),
+ ),
),
),
),
diff --git a/apps/server/src/serverRuntimeStartup.ts b/apps/server/src/serverRuntimeStartup.ts
--- a/apps/server/src/serverRuntimeStartup.ts
+++ b/apps/server/src/serverRuntimeStartup.ts
@@ -388,8 +388,22 @@
yield* Effect.logInfo("Authentication required. Open T3 Code using the pairing URL.", {
pairingUrl,
});
+ if (!serverConfig.noBrowser) {
+ const { openBrowser } = yield* Open;
+ yield* runStartupPhase(
+ "browser.open",
+ openBrowser(pairingUrl).pipe(
+ Effect.catch(() =>
+ Effect.logInfo("browser auto-open unavailable", {
+ hint: `Open ${pairingUrl} in your browser.`,
+ }),
+ ),
+ ),
+ );
+ }
+ } else {
+ yield* runStartupPhase("browser.open", maybeOpenBrowser);
}
- yield* runStartupPhase("browser.open", maybeOpenBrowser);
yield* Effect.logDebug("startup phase: complete");
}),
);You can send follow-ups to the cloud agent here.
ApprovabilityVerdict: Needs human review This PR introduces new authentication infrastructure including bootstrap credentials, session management, and pairing flows - explicitly security-sensitive code that requires human review. Additionally, there are multiple unresolved review comments identifying security concerns including session tokens being exposed in response bodies alongside httpOnly cookies, and missing secure flags on cookies for remote environments. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Auth bootstrap fetch uses unsupported ws:// protocol URL
- Added a wsUrlToHttpUrl() helper that converts ws:/wss: protocols to http:/https: and applied it at all three call sites where resolvePrimaryEnvironmentBootstrapUrl() is passed to fetch().
Or push these changes by commenting:
@cursor push 59ffed3254
Preview (59ffed3254)
diff --git a/apps/web/src/authBootstrap.test.ts b/apps/web/src/authBootstrap.test.ts
--- a/apps/web/src/authBootstrap.test.ts
+++ b/apps/web/src/authBootstrap.test.ts
@@ -86,10 +86,10 @@
expect(fetchMock).toHaveBeenCalledTimes(2);
expect(fetchMock.mock.calls[0]?.[0]).toEqual(
- new URL("/api/auth/session", "ws://localhost:3773/"),
+ new URL("/api/auth/session", "http://localhost:3773/"),
);
expect(fetchMock.mock.calls[1]?.[0]).toEqual(
- new URL("/api/auth/bootstrap", "ws://localhost:3773/"),
+ new URL("/api/auth/bootstrap", "http://localhost:3773/"),
);
});
diff --git a/apps/web/src/authBootstrap.ts b/apps/web/src/authBootstrap.ts
--- a/apps/web/src/authBootstrap.ts
+++ b/apps/web/src/authBootstrap.ts
@@ -2,6 +2,13 @@
import { resolvePrimaryEnvironmentBootstrapUrl } from "./environmentBootstrap";
+function wsUrlToHttpUrl(url: string): string {
+ const parsed = new URL(url);
+ if (parsed.protocol === "ws:") parsed.protocol = "http:";
+ else if (parsed.protocol === "wss:") parsed.protocol = "https:";
+ return parsed.href;
+}
+
export type ServerAuthGateState =
| { status: "authenticated" }
| {
@@ -80,7 +87,7 @@
}
async function bootstrapServerAuth(): Promise<ServerAuthGateState> {
- const baseUrl = resolvePrimaryEnvironmentBootstrapUrl();
+ const baseUrl = wsUrlToHttpUrl(resolvePrimaryEnvironmentBootstrapUrl());
const bootstrapCredential = getBootstrapCredential();
const currentSession = await fetchSessionState(baseUrl);
if (currentSession.authenticated) {
@@ -112,7 +119,10 @@
throw new Error("Enter a pairing token to continue.");
}
- await exchangeBootstrapCredential(resolvePrimaryEnvironmentBootstrapUrl(), trimmedCredential);
+ await exchangeBootstrapCredential(
+ wsUrlToHttpUrl(resolvePrimaryEnvironmentBootstrapUrl()),
+ trimmedCredential,
+ );
stripPairingTokenFromUrl();
}You can send follow-ups to the cloud agent here.
- Decrement remaining uses atomically on consume - Clean up temp files without masking secret persistence errors
- Convert ws/wss environment URLs to fetchable http/https bases - Cover remote pairing auth bootstrap and URL resolution with tests
- Treat missing-file removals as success - Wrap other remove failures in SecretStoreError - Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Cached auth state not invalidated after successful pairing
- Added
bootstrapPromise = nullafter the successful credential exchange insubmitServerAuthCredential, so subsequent calls toresolveInitialServerAuthGateStatere-evaluate the auth state instead of returning the stale cached requires-auth promise.
- Added
Or push these changes by commenting:
@cursor push 264a9be49a
Preview (264a9be49a)
diff --git a/apps/web/src/authBootstrap.ts b/apps/web/src/authBootstrap.ts
--- a/apps/web/src/authBootstrap.ts
+++ b/apps/web/src/authBootstrap.ts
@@ -123,6 +123,7 @@
await exchangeBootstrapCredential(resolvePrimaryEnvironmentHttpBaseUrl(), trimmedCredential);
stripPairingTokenFromUrl();
+ bootstrapPromise = null;
}
export function resolveInitialServerAuthGateState(): Promise<ServerAuthGateState> {You can send follow-ups to the cloud agent here.
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
|
|
||
| if (!response.ok) { | ||
| const message = await response.text(); | ||
| throw new Error(message || `Failed to bootstrap auth session (${response.status}).`); |
There was a problem hiding this comment.
Error response displayed as raw JSON string
Medium Severity
When the bootstrap credential exchange fails, exchangeBootstrapCredential reads the error response body with response.text(), yielding the raw JSON string {"error":"Invalid bootstrap credential."}. This string is then surfaced as the user-facing error message on the pairing screen via errorMessageFromUnknown. The server's toUnauthorizedResponse sends a JSON body, so the client needs to parse it and extract the .error field instead of using the raw text.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a2e4818. Configure here.
| } catch { | ||
| return baseUrl; | ||
| } | ||
| } |
There was a problem hiding this comment.
HTTP base URL retains WebSocket path component
Low Severity
getKnownEnvironmentHttpBaseUrl converts the protocol from ws:/wss: to http:/https: but preserves the full path (e.g., ws://host:port/ws → http://host:port/ws). Current callers use new URL("/absolute/path", baseUrl) which replaces the path entirely, making this safe today. However, the function name implies a "base URL" suitable for constructing endpoints, and any future caller using string concatenation (like ${baseUrl}/api/...) would produce incorrect URLs like http://host:port/ws/api/....
Reviewed by Cursor Bugbot for commit a2e4818. Configure here.
- Clarify auth policy, bootstrap, and session method meanings - Describe the server auth descriptor used by clients - Add guidance for pairing and steady-state auth flows - Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 5 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for 2 of the 3 issues found in the latest run.
- ✅ Fixed: Token split allows extra segments to pass verification
- Added a check that
token.split(".")produces exactly 2 parts before destructuring, rejecting tokens with extra segments.
- Added a check that
- ✅ Fixed: Secret store getOrCreateRandom has TOCTOU race condition
- Wrapped the read-then-write sequence in
getOrCreateRandomwith aSemaphore(1)mutex to ensure atomicity.
- Wrapped the read-then-write sequence in
Or push these changes by commenting:
@cursor push f69311f5db
Preview (f69311f5db)
diff --git a/apps/server/src/auth/Layers/ServerSecretStore.ts b/apps/server/src/auth/Layers/ServerSecretStore.ts
--- a/apps/server/src/auth/Layers/ServerSecretStore.ts
+++ b/apps/server/src/auth/Layers/ServerSecretStore.ts
@@ -1,6 +1,6 @@
import * as Crypto from "node:crypto";
-import { Effect, FileSystem, Layer, Path } from "effect";
+import { Effect, FileSystem, Layer, Path, Semaphore } from "effect";
import * as PlatformError from "effect/PlatformError";
import { ServerConfig } from "../../config.ts";
@@ -60,6 +60,8 @@
);
};
+ const mutex = yield* Semaphore.make(1);
+
const getOrCreateRandom: ServerSecretStoreShape["getOrCreateRandom"] = (name, bytes) =>
get(name).pipe(
Effect.flatMap((existing) => {
@@ -70,6 +72,7 @@
const generated = Crypto.randomBytes(bytes);
return set(name, generated).pipe(Effect.as(Uint8Array.from(generated)));
}),
+ mutex.withPermits(1),
);
const remove: ServerSecretStoreShape["remove"] = (name) =>
diff --git a/apps/server/src/auth/Layers/SessionCredentialService.ts b/apps/server/src/auth/Layers/SessionCredentialService.ts
--- a/apps/server/src/auth/Layers/SessionCredentialService.ts
+++ b/apps/server/src/auth/Layers/SessionCredentialService.ts
@@ -56,7 +56,13 @@
});
const verify: SessionCredentialServiceShape["verify"] = Effect.fn("verify")(function* (token) {
- const [encodedPayload, signature] = token.split(".");
+ const parts = token.split(".");
+ if (parts.length !== 2) {
+ return yield* new SessionCredentialError({
+ message: "Malformed session token.",
+ });
+ }
+ const [encodedPayload, signature] = parts;
if (!encodedPayload || !signature) {
return yield* new SessionCredentialError({
message: "Malformed session token.",You can send follow-ups to the cloud agent here.
| return yield* new SessionCredentialError({ | ||
| message: "Malformed session token.", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Token split allows extra segments to pass verification
Low Severity
The verify function uses token.split(".") without limiting the result to exactly two segments. A token like payload.validSig.arbitraryData would still pass verification because only the first two segments are destructured and the rest are silently ignored. The HMAC is verified against just the first segment, so extra segments have no effect on the signature check. This violates strict token format validation — the function accepts malformed tokens without rejecting them.
Reviewed by Cursor Bugbot for commit be42c17. Configure here.
| const generated = Crypto.randomBytes(bytes); | ||
| return set(name, generated).pipe(Effect.as(Uint8Array.from(generated))); | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Secret store getOrCreateRandom has TOCTOU race condition
Low Severity
The getOrCreateRandom function performs a non-atomic read-then-write sequence via separate get and set calls. If two fibers call this concurrently, both could observe null from get, both generate different random secrets, and the last writer wins — leaving the first fiber holding a secret that doesn't match what's persisted on disk. This could cause session token verification failures if the signing key is overwritten mid-startup.
Reviewed by Cursor Bugbot for commit be42c17. Configure here.
| sessions.issue({ | ||
| method: "browser-session-cookie", | ||
| subject: grant.method, | ||
| }), |
There was a problem hiding this comment.
Bootstrap exchange always hardcodes browser-session-cookie method
Low Severity
In exchangeBootstrapCredential, the session is always issued with method: "browser-session-cookie" regardless of how the client will actually use the credential. When the session token is returned in the response body and used as a bearer token (e.g., for WebSocket ?token= query param auth), the session's method claim doesn't reflect the actual usage. The ServerAuthDescriptor advertises both "browser-session-cookie" and "bearer-session-token" as supported session methods, but only the former is ever issued.
Reviewed by Cursor Bugbot for commit be42c17. Configure here.
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
The bootstrap exchange sets the session cookie via Set-Cookie header, making browser-session-cookie the correct method; the token in the response body is a convenience copy, and at exchange time the server cannot know which transport the client will ultimately use.
You can send follow-ups to the cloud agent here.
- route auth and observability requests through the active web origin - simplify desktop startup for pairing mode and loopback backend binding - improve the boot shell while the app waits for the initial config snapshot
There was a problem hiding this comment.
🟢 Low
VITE_DEV_SERVER_URL is trimmed and validated by the parent on lines 8-17, but the child process receives the untrimmed value directly from process.env via childEnv. When the original environment variable contains whitespace, the parent accepts the URL after trimming while the child receives the raw value, causing URL parsing failures in the Electron app. Consider passing the validated devServerUrl to the child explicitly, or ensuring childEnv uses the trimmed value.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/desktop/scripts/dev-electron.mjs around line 65:
`VITE_DEV_SERVER_URL` is trimmed and validated by the parent on lines 8-17, but the child process receives the untrimmed value directly from `process.env` via `childEnv`. When the original environment variable contains whitespace, the parent accepts the URL after trimming while the child receives the raw value, causing URL parsing failures in the Electron app. Consider passing the validated `devServerUrl` to the child explicitly, or ensuring `childEnv` uses the trimmed value.
Evidence trail:
apps/desktop/scripts/dev-electron.mjs lines 8, 37, and 65-73 at REVIEWED_COMMIT:
- Line 8: `const devServerUrl = process.env.VITE_DEV_SERVER_URL?.trim();` (trimmed for validation)
- Line 37: `const childEnv = { ...process.env };` (spreads original untrimmed env)
- Lines 65-73: `spawn(..., { env: childEnv, ... })` (child receives untrimmed value)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 7 total unresolved issues (including 5 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Session token exposed in response body alongside HttpOnly cookie
- Stripped sessionToken from the bootstrap JSON response body by destructuring it out before serialization, so the token is only transmitted via the httpOnly cookie.
- ✅ Fixed: Session cookie missing
secureflag for non-loopback environments- Added conditional
secure: descriptor.policy === "remote-reachable"to the cookie options so the flag is set when the server is configured for remote access.
- Added conditional
Or push these changes by commenting:
@cursor push 21a3606f32
Preview (21a3606f32)
diff --git a/apps/server/src/auth/http.ts b/apps/server/src/auth/http.ts
--- a/apps/server/src/auth/http.ts
+++ b/apps/server/src/auth/http.ts
@@ -40,12 +40,14 @@
);
const result = yield* serverAuth.exchangeBootstrapCredential(payload.credential);
- return yield* HttpServerResponse.jsonUnsafe(result, { status: 200 }).pipe(
+ const { sessionToken: _token, ...responseBody } = result;
+ return yield* HttpServerResponse.jsonUnsafe(responseBody, { status: 200 }).pipe(
HttpServerResponse.setCookie(descriptor.sessionCookieName, result.sessionToken, {
expires: DateTime.toDate(result.expiresAt),
httpOnly: true,
path: "/",
sameSite: "lax",
+ secure: descriptor.policy === "remote-reachable",
}),
);
}).pipe(Effect.catchTag("AuthError", (error) => Effect.succeed(toUnauthorizedResponse(error)))),
diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts
--- a/apps/server/src/server.test.ts
+++ b/apps/server/src/server.test.ts
@@ -492,6 +492,12 @@
return `http://127.0.0.1:${address.port}${pathname}`;
});
+function parseSessionTokenFromSetCookie(setCookie: string | null): string | null {
+ if (!setCookie) return null;
+ const match = /t3_session=([^;]+)/.exec(setCookie);
+ return match?.[1] ?? null;
+}
+
const bootstrapBrowserSession = (credential = defaultDesktopBootstrapToken) =>
Effect.gen(function* () {
const bootstrapUrl = yield* getHttpServerUrl("/api/auth/bootstrap");
@@ -509,13 +515,15 @@
const body = (yield* Effect.promise(() => response.json())) as {
readonly authenticated: boolean;
readonly sessionMethod: string;
- readonly sessionToken: string;
readonly expiresAt: string;
};
+ const cookie = response.headers.get("set-cookie");
+ const sessionToken = parseSessionTokenFromSetCookie(cookie);
return {
response,
body,
- cookie: response.headers.get("set-cookie"),
+ cookie,
+ sessionToken,
};
});
@@ -525,18 +533,18 @@
return cachedDefaultSessionToken;
}
- const { response, body } = yield* bootstrapBrowserSession(credential);
- if (!response.ok) {
+ const { response, sessionToken } = yield* bootstrapBrowserSession(credential);
+ if (!response.ok || !sessionToken) {
return yield* Effect.fail(
new Error(`Expected bootstrap session response to succeed, got ${response.status}`),
);
}
if (credential === defaultDesktopBootstrapToken) {
- cachedDefaultSessionToken = body.sessionToken;
+ cachedDefaultSessionToken = sessionToken;
}
- return body.sessionToken;
+ return sessionToken;
});
const getWsServerUrl = (
@@ -720,13 +728,15 @@
Effect.gen(function* () {
yield* buildAppUnderTest();
- const { response: bootstrapResponse, body: bootstrapBody } = yield* bootstrapBrowserSession();
+ const { response: bootstrapResponse, sessionToken: bootstrapSessionToken } =
+ yield* bootstrapBrowserSession();
assert.equal(bootstrapResponse.status, 200);
+ assert(bootstrapSessionToken, "Expected session token in Set-Cookie header");
const wsUrl = appendSessionTokenToUrl(
yield* getWsServerUrl("/ws", { authenticated: false }),
- bootstrapBody.sessionToken,
+ bootstrapSessionToken,
);
const response = yield* Effect.scoped(
withWsRpcClient(wsUrl, (client) => client[WS_METHODS.serverGetConfig]({})),
diff --git a/apps/web/src/authBootstrap.test.ts b/apps/web/src/authBootstrap.test.ts
--- a/apps/web/src/authBootstrap.test.ts
+++ b/apps/web/src/authBootstrap.test.ts
@@ -65,7 +65,6 @@
jsonResponse({
authenticated: true,
sessionMethod: "browser-session-cookie",
- sessionToken: "session-token",
expiresAt: "2026-04-05T00:00:00.000Z",
}),
);
@@ -207,7 +206,6 @@
jsonResponse({
authenticated: true,
sessionMethod: "browser-session-cookie",
- sessionToken: "session-token",
expiresAt: "2026-04-05T00:00:00.000Z",
}),
);
diff --git a/apps/web/src/authBootstrap.ts b/apps/web/src/authBootstrap.ts
--- a/apps/web/src/authBootstrap.ts
+++ b/apps/web/src/authBootstrap.ts
@@ -1,4 +1,4 @@
-import type { AuthBootstrapInput, AuthBootstrapResult, AuthSessionState } from "@t3tools/contracts";
+import type { AuthBootstrapInput, AuthSessionState } from "@t3tools/contracts";
import { resolveServerHttpUrl } from "./lib/utils";
export type ServerAuthGateState =
@@ -56,7 +56,7 @@
return (await response.json()) as AuthSessionState;
}
-async function exchangeBootstrapCredential(credential: string): Promise<AuthBootstrapResult> {
+async function exchangeBootstrapCredential(credential: string): Promise<void> {
const payload: AuthBootstrapInput = { credential };
const response = await fetch(resolveServerHttpUrl({ pathname: "/api/auth/bootstrap" }), {
body: JSON.stringify(payload),
@@ -71,8 +71,6 @@
const message = await response.text();
throw new Error(message || `Failed to bootstrap auth session (${response.status}).`);
}
-
- return (await response.json()) as AuthBootstrapResult;
}
async function bootstrapServerAuth(): Promise<ServerAuthGateState> {
diff --git a/apps/web/test/authHttpHandlers.ts b/apps/web/test/authHttpHandlers.ts
--- a/apps/web/test/authHttpHandlers.ts
+++ b/apps/web/test/authHttpHandlers.ts
@@ -2,7 +2,6 @@
import { HttpResponse, http } from "msw";
const TEST_SESSION_EXPIRES_AT = "2026-05-01T12:00:00.000Z";
-const TEST_SESSION_TOKEN = "browser-test-session-token";
export function createAuthenticatedSessionHandlers(getAuthDescriptor: () => ServerAuthDescriptor) {
return [
@@ -18,7 +17,6 @@
HttpResponse.json({
authenticated: true,
sessionMethod: "browser-session-cookie",
- sessionToken: TEST_SESSION_TOKEN,
expiresAt: TEST_SESSION_EXPIRES_AT,
}),
),You can send follow-ups to the cloud agent here.
| path: "/", | ||
| sameSite: "lax", | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Session token exposed in response body alongside HttpOnly cookie
Medium Severity
The /api/auth/bootstrap response returns the raw sessionToken in the JSON body while simultaneously setting it as an httpOnly cookie. The httpOnly flag is meant to protect the session credential from JavaScript access (e.g., XSS), but including the same token in the response body completely negates that protection — any script on the page can read it from the fetch response. If the token is needed for non-cookie auth (bearer/WebSocket), it would be better to issue a separate bearer-only token for that purpose.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit dca54c7. Configure here.
| httpOnly: true, | ||
| path: "/", | ||
| sameSite: "lax", | ||
| }), |
There was a problem hiding this comment.
Session cookie missing secure flag for non-loopback environments
Medium Severity
The session cookie set by the bootstrap endpoint omits the secure flag entirely. While this is fine for local development over HTTP, the auth model explicitly supports remote-reachable environments where TLS is expected. Without secure, the cookie could be sent over plaintext HTTP on a remote/tunneled connection, exposing the session token to network eavesdropping. The flag could be set conditionally based on the auth policy or the request protocol.
Reviewed by Cursor Bugbot for commit dca54c7. Configure here.
- Replace the verbose boot shell with a minimal splash screen - Move pairing loading UI into the /pair route pending state - Gate the main app on bootstrap completion instead of auth preload
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 10 total unresolved issues (including 7 from previous reviews).
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues. You can view the agent here.
Reviewed by Cursor Bugbot for commit 5a21931. Configure here.
| }), | ||
| remainingUses: 1, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Desktop bootstrap token expires before backend ready
Medium Severity
The desktop bootstrap token is seeded with a 5-minute TTL (DEFAULT_ONE_TIME_TOKEN_TTL_MINUTES) computed from DateTime.now at service construction time. However, the desktop main process generates backendBootstrapToken before spawning the backend, and the token isn't exchanged until the renderer window loads and the auth bootstrap completes. If backend startup, window creation, or waitForBackendHttpReady is slow (e.g. heavy machine, or the 10-second readiness timeout fires), the 5-minute window measured from server layer construction could expire before the renderer exchanges it. A longer TTL or one anchored to when the server is actually ready to accept connections would be safer.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5a21931. Configure here.
| return "Disconnected from T3 Server"; | ||
| } | ||
|
|
||
| function buildReconnectTitle(_status: WsConnectionStatus): string { |
There was a problem hiding this comment.
Unused parameter in buildReconnectTitle after refactoring
Low Severity
buildReconnectTitle previously had a conditional branch using the status parameter (checking status.nextRetryAt). That branch was removed, but the function still accepts _status as a parameter. The function now unconditionally returns a static string, making it an unnecessary wrapper that could be inlined or simplified.
Reviewed by Cursor Bugbot for commit 5a21931. Configure here.
| repositoryIdentity: true, | ||
| }, | ||
| }; | ||
| let cachedDefaultSessionToken: string | null = null; |
There was a problem hiding this comment.
Module-level shared mutable state across parallel test runs
Medium Severity
cachedDefaultSessionToken is a module-level mutable variable used to cache the session token across multiple test helpers. Since buildAppUnderTest resets it to null (line 296), but each test builds a new server instance with a fresh signing key, a stale cached token from a prior test could be used if getAuthenticatedSessionToken is called before buildAppUnderTest in a test, or if test ordering assumptions are violated. More critically, the one-time bootstrap token can only be consumed once per server instance, so caching across calls within the same test works, but the pattern is fragile.
Reviewed by Cursor Bugbot for commit 5a21931. Configure here.
1 participant
Summary
/pairroute for browser pairingbeforeLoad, add a first-paint loading shell, and document the auth architectureTesting
Note
High Risk
Introduces a new server-wide auth layer (bootstrap exchange + signed sessions) and enforces it across HTTP routes and WebSocket upgrades, replacing the legacy
?token=flow; mismatches between clients, cookies, and auth gating can break connectivity if any path is missed.Overview
Adds a server-wide authentication system based on one-time bootstrap credentials exchanged for signed session tokens (cookie/bearer), with new endpoints
GET /api/auth/sessionandPOST /api/auth/bootstrapand a persistent secret store understateDir/secrets.Enforces auth on previously open surfaces (WebSocket
/wsupgrade plus HTTP routes like attachments, project favicon, and OTLP trace proxy), removes the legacyauthTokenconfig/env/CLI flag in favor of adesktopBootstrapToken, and updates desktop startup to pass bootstrap data and wait for backend HTTP readiness.Updates the web app to gate routes via router
beforeLoad, adds a dedicated/pairflow that can auto-consume?token=or accept manual entry, introduces a first-paint splash shell + theme preflight, and adjusts URL helpers/tests to useresolveServerHttpUrlfor same-origin HTTP in local dev while keeping WS endpoints unchanged.Reviewed by Cursor Bugbot for commit 5a21931. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Implement server auth bootstrap and pairing flow with session-based authentication
/api/auth/sessionand/api/auth/bootstrapendpoints, issues signed session tokens, and manages one-time bootstrap credentials with atomic single-use semanticsServerAuth,SessionCredentialService,BootstrapCredentialService,ServerSecretStore, andServerAuthPolicyEffect services on the server, with secrets persisted to disk under a newsecretsDirpath/attachments/*,/api/project-favicon,/api/observability/v1/traces) now require session authentication; the previous staticauthToken/--auth-tokenmechanism is removed/pairroute in the web app that auto-submits a one-time token from the URL query string or prompts for manual credential entry; unauthenticated access to/_chatand/settingsredirects there/api/auth/sessionfor readiness, and seeds adesktopBootstrapTokeninstead of the prior auth token;ELECTRON_RENDERER_PORTandT3CODE_AUTH_TOKENenv vars are removedauthToken/T3CODE_AUTH_TOKEN/--auth-tokenare fully removed; existing integrations relying on query-parameter token auth or the old env var will no longer authenticateMacroscope summarized 5a21931.