Skip to content

Commit 2cc83ee

Browse files
arul28claude
andauthored
harden(account): packaged builds ignore development Clerk config (#851)
* harden(account): ignore development Clerk config in packaged builds A developer project that sets CLERK_ISSUER/CLERK_JWKS_URL/ CLERK_OAUTH_CLIENT_ID (or ADE_ACCOUNT_DIRECTORY_URL) to ADE's development Clerk instance leaked into the packaged production app's brain, which then authenticated against dev Clerk and got a 401 "invalid token" from the production account directory. Packaged builds now set ADE_RUNTIME_PACKAGED=1 (bootstrap.ts via the existing source-checkout check; desktop main.ts via app.isPackaged). When set, the account resolvers discard any Clerk override that resolves to a development instance (*.clerk.accounts.dev, incl. trailing-dot aliases) or the development account directory — atomically snapping issuer, JWKS, client, and directory back to the built-in production defaults (never a hybrid) and logging one warning. The machine publisher's directory override is guarded the same way. Source-checkout/dev builds are unchanged; ADE_ALLOW_DEVELOPMENT_CLERK=1 is a deliberate escape hatch. Non-development custom issuer overrides are still honored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(account): fix packaged detection + close directory guard gaps Independent review findings: - Packaged detection keyed off the module filename (bootstrap.cjs), but bootstrap is bundled into main.cjs/cli.cjs, so dev builds running built bundles misidentified as packaged and would auth against production Clerk. Extract isSourceCheckoutRuntimeModule to runtimePackaging.ts and make it directory-based (matches an apps/{ade-cli,desktop}/{src,dist}/ path segment) — packaged resources have no such segment. - Route accountMachineDirectoryService's raw ADE_ACCOUNT_DIRECTORY_URL env fallback through the dev-directory guard (was a latent bypass). - Match the development account directory by host, so a dev-host URL with an extra path/subdomain is also caught. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(account): also reject the development OAuth client id The packaged-build enforcement discarded dev issuers and dev JWKS hosts but not the development OAuth client id, so a custom issuer paired with DEVELOPMENT_ADE_CLERK_OAUTH_CLIENT_ID survived. Treat the dev client id as an independent development-instance marker in both enforcement functions, replacing the whole tuple with production defaults. Tests cover a dev client id behind a custom issuer/JWKS for OAuth and attestation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(account): derive sign-in availability from the hardened resolver isLoginConfigured() read the raw CLERK_* values, so in a packaged build a stale partial development secret (issuer or client id only) reported configured=false and disabled the Account-page sign-in — even though the hardened resolver would fall back to production and log in fine. Derive it from the same resolveAccountOAuthConfig() the login flow uses (which applies the packaged development-Clerk-ignore policy), and drop the now dead readProjectSecret helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(account): reject stored development sessions in packaged builds The config resolvers were hardened, but a stored account.session.v1 created against dev Clerk bypassed them: getAccessToken() returned the stored dev access token directly and refreshed against the stored dev oauthConfig, so a packaged build with a pre-existing dev session kept hitting the production directory with a dev token (401) until manual sign-out. In packaged mode (and equally for ADE_ACCOUNT_TOKEN credentials), reject any session whose stored oauthConfig issuer/client is a development instance or whose access-token `iss` claim is a dev host, before token return, refresh, userinfo, or directory use. A persisted dev session is compare-and-deleted from the shared credential store (only when the stored value still matches what we read) so status flips to signed-out without erasing a newer peer-written session, and the user re-signs-in against production. ADE_ALLOW_DEVELOPMENT_CLERK=1 still keeps dev working; source checkouts are unchanged. Swept the whole account surface (getAccessToken, refresh, status, userinfo, env-token, publisher, directory) for equivalent bypasses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(account): make dev-session invalidation race-safe and signed-out Two review findings on the stored-session rejection: - The updateSync-less fallback in invalidateStoredSessionIfCurrent did a non-atomic getSync-then-deleteSync, which could delete a production credential a peer wrote between the two calls. Drop the non-atomic delete: use atomic compare-and-delete when available, otherwise leave the value in place and reject the development session on every read. - A rejected development ADE_ACCOUNT_TOKEN reported source: "env-token" with signedIn: false, so downstream treated it as an active env credential and blocked production re-auth. Report a fully signed-out status (source: null) instead. Adds coverage for a store without atomic compare-and-delete (rejected, not clobbered). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(account): unblock re-auth and honor the post-invalidation retry Two final review findings on the packaged dev-Clerk policy: - startLogin()/startDeviceLogin()/createToken() blocked whenever a raw ADE_ACCOUNT_TOKEN was present, so after a rejected development token correctly reported signed-out, Sign In threw "already providing" instead of starting production login. Route every login-gating path through readAcceptedEnvCredential(), which treats a rejected development credential as absent, so production login proceeds. - readSession() returned null right after invalidating a development session, ignoring the promised retry: when the atomic compare-and-delete saw a peer had already replaced it with a production session, that record stayed but the read still reported signed-out. Re-read once and return the peer-written production session in the same call; only a still-rejectable or absent session yields signed-out. Retry at most once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3dbab90 commit 2cc83ee

19 files changed

Lines changed: 1377 additions & 95 deletions

apps/ade-cli/src/bootstrap.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,25 @@
11
import { describe, expect, it, vi } from "vitest";
22
import { createEventBuffer, type BufferedEvent } from "./eventBuffer";
33
import { createPrEventFanout } from "./prEventFanout";
4+
import { isSourceCheckoutRuntimeModule } from "./runtimePackaging";
5+
6+
describe("isSourceCheckoutRuntimeModule", () => {
7+
it.each([
8+
"/Users/developer/ADE/apps/ade-cli/src/bootstrap.ts",
9+
"/Users/developer/ADE/apps/ade-cli/dist/cli.cjs",
10+
"/Users/developer/ADE/apps/ade-cli/dist/bootstrap.cjs",
11+
"/Users/developer/ADE/apps/desktop/dist/main/main.cjs",
12+
])("classifies a source-checkout module as development: %s", (modulePath) => {
13+
expect(isSourceCheckoutRuntimeModule(modulePath)).toBe(true);
14+
});
15+
16+
it.each([
17+
"/Applications/ADE.app/Contents/Resources/app.asar/dist/main/main.cjs",
18+
"/Applications/ADE.app/Contents/Resources/ade-cli/cli.cjs",
19+
])("classifies a packaged module as packaged: %s", (modulePath) => {
20+
expect(isSourceCheckoutRuntimeModule(modulePath)).toBe(false);
21+
});
22+
});
423

524
describe("createPrEventFanout", () => {
625
const event = {

apps/ade-cli/src/bootstrap.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import os from "node:os";
44
import path from "node:path";
55
import { fileURLToPath } from "node:url";
66
import * as nodePty from "node-pty";
7+
import { isSourceCheckoutRuntimeModule } from "./runtimePackaging";
78
import { createFileLogger, type Logger } from "../../desktop/src/main/services/logging/logger";
89
import { classifySqliteOpenError, openKvDb, type AdeDb } from "../../desktop/src/main/services/state/kvDb";
910
import {
@@ -307,13 +308,16 @@ export function ensureAdePaths(projectRoot: string): AdeRuntimePaths {
307308
};
308309
}
309310

310-
function isSourceCheckoutRuntimeModule(modulePath: string): boolean {
311-
return /[/\\]apps[/\\]ade-cli[/\\](?:src|dist)[/\\]bootstrap\.(?:ts|js|cjs)$/i.test(modulePath);
312-
}
313-
314311
const currentModulePath =
315312
typeof __filename === "string" ? __filename : fileURLToPath(import.meta.url);
316313

314+
if (
315+
!isSourceCheckoutRuntimeModule(currentModulePath)
316+
&& process.env.ADE_RUNTIME_PACKAGED === undefined
317+
) {
318+
process.env.ADE_RUNTIME_PACKAGED = "1";
319+
}
320+
317321
function automationsEnabledForHeadlessRuntime(): boolean {
318322
const override = readAutomationsEnvOverride(process.env);
319323
if (override !== null) return override;

apps/ade-cli/src/cli.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ import {
3535
summarizeExecution,
3636
unwrapToolResult,
3737
} from "./cli";
38+
import {
39+
DEVELOPMENT_ADE_CLERK_ISSUER,
40+
DEVELOPMENT_ADE_CLERK_OAUTH_CLIENT_ID,
41+
} from "../../desktop/src/shared/accountDirectory";
3842
import { generateRpcAuthToken } from "./rpcAuth";
3943
import { JsonRpcClient } from "./tuiClient/jsonRpcClient";
4044
import { EncryptedFileCredentialStore } from "./services/credentials/credentialStore";
@@ -344,6 +348,41 @@ describe("ADE CLI", () => {
344348
.toBe("loopback");
345349
});
346350

351+
it("treats rejected packaged development env credentials as absent when selecting login mode", () => {
352+
const developmentAccessToken = [
353+
Buffer.from(JSON.stringify({ alg: "none", typ: "JWT" })).toString("base64url"),
354+
Buffer.from(JSON.stringify({
355+
iss: DEVELOPMENT_ADE_CLERK_ISSUER,
356+
sub: "development-user",
357+
exp: Math.floor(Date.now() / 1000) + 3_600,
358+
})).toString("base64url"),
359+
"signature",
360+
].join(".");
361+
const developmentRefreshToken = `ade_account_v1.${Buffer.from(JSON.stringify({
362+
version: 1,
363+
refreshToken: "development-refresh-token",
364+
issuer: DEVELOPMENT_ADE_CLERK_ISSUER,
365+
clientId: DEVELOPMENT_ADE_CLERK_OAUTH_CLIENT_ID,
366+
}), "utf8").toString("base64url")}`;
367+
368+
for (const credential of [developmentAccessToken, developmentRefreshToken]) {
369+
const env = {
370+
ADE_RUNTIME_PACKAGED: "1",
371+
ADE_ACCOUNT_TOKEN: credential,
372+
DISPLAY: ":0",
373+
} as NodeJS.ProcessEnv;
374+
expect(detectAccountLoginMode({ env, platform: "linux" })).toBe("loopback");
375+
expect(detectAccountLoginMode({
376+
env: { ...env, SSH_CONNECTION: "host details" },
377+
platform: "linux",
378+
})).toBe("device");
379+
expect(detectAccountLoginMode({
380+
env: { ...env, ADE_ALLOW_DEVELOPMENT_CLERK: "1" },
381+
platform: "linux",
382+
})).toBe("env-token");
383+
}
384+
});
385+
347386
it("formats account auth sources and durable-token provisioning guidance", () => {
348387
expect(formatOutput({
349388
signedIn: true,

apps/ade-cli/src/cli.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ import type { AdeRuntime } from "./bootstrap";
102102
import { reseedBundledAdeSkillsForCli } from "./bootstrap";
103103
import { EncryptedFileCredentialStore } from "./services/credentials/credentialStore";
104104
import type { AccountMachinePublisherService } from "./services/account/accountMachinePublisherService";
105+
import { shouldRejectDevelopmentEnvCredential } from "./services/account/accountAuthService";
105106
import { DEFAULT_SYNC_HOST_PORT } from "./services/sync/syncProtocol";
106107
import {
107108
runAdeCodeRemote,
@@ -18213,7 +18214,13 @@ export function detectAccountLoginMode(args: {
1821318214
} = {}): AccountLoginMode {
1821418215
const env = args.env ?? process.env;
1821518216
if (args.explicitHeadless) return "device";
18216-
if (env.ADE_ACCOUNT_TOKEN?.trim()) return "env-token";
18217+
const envCredential = env.ADE_ACCOUNT_TOKEN?.trim();
18218+
if (
18219+
envCredential
18220+
&& !shouldRejectDevelopmentEnvCredential(env, envCredential)
18221+
) {
18222+
return "env-token";
18223+
}
1821718224
if (args.browserOpenFailed) return "device";
1821818225
if (env.SSH_TTY?.trim() || env.SSH_CONNECTION?.trim() || env.SSH_CLIENT?.trim()) {
1821918226
return "device";
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export function isSourceCheckoutRuntimeModule(modulePath: string): boolean {
2+
return /(?:^|[/\\])apps[/\\](?:ade-cli|desktop)[/\\](?:src|dist)[/\\]/i.test(
3+
modulePath,
4+
);
5+
}

0 commit comments

Comments
 (0)