Skip to content

Commit d3303bf

Browse files
committed
fix(codex-auth): scope duplicate checks by plan bucket
1 parent f62207d commit d3303bf

4 files changed

Lines changed: 53 additions & 30 deletions

File tree

devlog/_plan/260624_codex-multi-auth-security-implementation/40_phase40-manual-import-disable.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ Required imports:
219219
Assertions:
220220

221221
- `checkAccountIdCollision("shared-team-account", "member-b@example.test")` returns no collision when an existing pool account has the same ChatGPT account id but a different email;
222-
- Correction 2026-06-27: `checkAccountIdCollision("shared-team-account", "MEMBER-A@example.test")` also returns no collision. ChatGPT account id plus email is not an authoritative duplicate key because one user can legitimately hold both personal and business subscriptions. Duplicate prevention remains scoped to local alias checks and exact OAuth refresh-grant fingerprint matches.
222+
- Correction 2026-06-27: `checkAccountIdCollision("shared-team-account", "MEMBER-A@example.test", "business")` returns no collision when the existing account is personal. Personal and workspace subscriptions are separate duplicate buckets because one user can legitimately hold both. Within the same bucket, the existing ChatGPT account id plus normalized email collision guard still applies.
223223

224224
### MODIFY `src/codex-auth-api.ts` OAuth wording
225225

src/codex-auth-api.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,9 @@ export async function handleCodexAuthAPI(
214214
if (accounts.some(a => a.id === body.id) || getCodexAccountCredential(body.id)) {
215215
return jsonResponse({ error: `Account id already exists: ${body.id}` }, 400);
216216
}
217-
// 1.1: Duplicate check uses local alias plus exact credential material, not account identity.
217+
// 1.1: Duplicate check is scoped by personal vs workspace plan bucket.
218218
const derivedAccountId = extractAccountId(undefined, body.accessToken) ?? body.chatgptAccountId;
219-
const collision = checkAccountIdCollision(derivedAccountId, body.email, body.refreshToken);
219+
const collision = checkAccountIdCollision(derivedAccountId, body.email, body.plan);
220220
if (collision.collision) {
221221
return jsonResponse({ error: collision.reason }, 400);
222222
}
@@ -414,7 +414,6 @@ export async function handleCodexAuthAPI(
414414
const { getCredential } = await import("./oauth/store");
415415
const cred = getCredential("chatgpt");
416416
if (cred) {
417-
// 1.2: Duplicate check uses local alias plus exact credential material, not account identity.
418417
const oauthAccountId = cred.accountId;
419418
if (!oauthAccountId) {
420419
codexAuthLoginState.set(flowId, {
@@ -425,14 +424,6 @@ export async function handleCodexAuthAPI(
425424
completed = true;
426425
break;
427426
}
428-
const collision = checkAccountIdCollision(oauthAccountId, cred.email, cred.refresh);
429-
if (collision.collision) {
430-
codexAuthLoginState.set(flowId, {
431-
status: "error", error: collision.reason, doneAt: Date.now(),
432-
});
433-
completed = true;
434-
break;
435-
}
436427

437428
let email = cred.email || accountId;
438429
let plan: string | undefined;
@@ -450,6 +441,15 @@ export async function handleCodexAuthAPI(
450441
quota = parseUsageQuota(data);
451442
}
452443
} catch { /* wham fetch is non-blocking */ }
444+
// 1.2: Duplicate check is scoped by personal vs workspace plan bucket.
445+
const collision = checkAccountIdCollision(oauthAccountId, email, plan);
446+
if (collision.collision) {
447+
codexAuthLoginState.set(flowId, {
448+
status: "error", error: collision.reason, doneAt: Date.now(),
449+
});
450+
completed = true;
451+
break;
452+
}
453453

454454
saveCodexAccountCredential(accountId, {
455455
accessToken: cred.access,

src/codex-auth-collision.ts

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { existsSync, readFileSync } from "node:fs";
22
import { join } from "node:path";
33
import os from "node:os";
4-
import { loadCodexAccountStore, refreshGrantFingerprintForToken } from "./codex-account-store";
4+
import { getCodexAccountCredential } from "./codex-account-store";
5+
import { loadConfig } from "./config";
56
import { extractAccountId } from "./oauth/chatgpt";
67

78
export function readCodexTokens(): { access_token: string; account_id: string; id_token?: string } | null {
@@ -27,19 +28,31 @@ export function getMainChatgptAccountId(): string | null {
2728
return extractAccountId(tokens.id_token, tokens.access_token) ?? (tokens.account_id || null);
2829
}
2930

30-
// ChatGPT account ids and emails are not authoritative duplicate keys:
31-
// one user can legitimately hold both personal and business subscriptions.
32-
// Only the local alias and exact OAuth grant material are duplicate keys.
31+
function normalizedEmail(email: string | undefined | null): string | null {
32+
const trimmed = email?.trim().toLowerCase();
33+
return trimmed || null;
34+
}
35+
36+
function isWorkspacePlan(plan: string | undefined | null): boolean {
37+
return !!plan && /team|business|enterprise|workspace|edu/i.test(plan);
38+
}
39+
40+
// Personal and workspace subscriptions are separate duplicate buckets.
41+
// Within each bucket, keep the original ChatGPT account id + email collision guard.
3342
export function checkAccountIdCollision(
34-
_chatgptAccountId: string,
35-
_email?: string | null,
36-
refreshToken?: string | null,
43+
chatgptAccountId: string,
44+
email?: string | null,
45+
plan?: string | null,
3746
): { collision: true; reason: string } | { collision: false } {
38-
if (!refreshToken) return { collision: false };
39-
const candidateFingerprint = refreshGrantFingerprintForToken(refreshToken);
40-
for (const [poolId, cred] of Object.entries(loadCodexAccountStore())) {
41-
if (refreshGrantFingerprintForToken(cred.refreshToken) === candidateFingerprint) {
42-
return { collision: true, reason: `Credential is already in the pool (${poolId}).` };
47+
const candidateEmail = normalizedEmail(email);
48+
const candidateWorkspace = isWorkspacePlan(plan);
49+
for (const account of loadConfig().codexAccounts ?? []) {
50+
if (account.isMain) continue;
51+
if (isWorkspacePlan(account.plan) !== candidateWorkspace) continue;
52+
const cred = getCodexAccountCredential(account.id);
53+
const poolEmail = normalizedEmail(account.email);
54+
if (cred && cred.chatgptAccountId === chatgptAccountId && (!candidateEmail || !poolEmail || poolEmail === candidateEmail)) {
55+
return { collision: true, reason: `Account is already in the pool (${account.id}).` };
4356
}
4457
}
4558
return { collision: false };

tests/codex-auth-collision.test.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ afterEach(() => {
2828
if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true });
2929
});
3030

31-
function seedAccount(id: string, email: string, chatgptAccountId: string): OcxConfig {
31+
function seedAccount(id: string, email: string, chatgptAccountId: string, plan?: string): OcxConfig {
3232
const config: OcxConfig = {
3333
port: 10100,
3434
providers: {},
3535
defaultProvider: "openai",
36-
codexAccounts: [{ id, email, isMain: false }],
36+
codexAccounts: [{ id, email, plan, isMain: false }],
3737
};
3838
saveConfig(config);
3939
saveCodexAccountCredential(id, {
@@ -57,18 +57,28 @@ describe("codex auth account collision", () => {
5757
test("allows the same email and account id because personal and business subscriptions can coexist", async () => {
5858
seedAccount("team-member-a", "member-a@example.test", "shared-team-account");
5959

60-
expect(checkAccountIdCollision("shared-team-account", "MEMBER-A@example.test", "refresh-personal-business")).toEqual({
60+
expect(checkAccountIdCollision("shared-team-account", "MEMBER-A@example.test", "business")).toEqual({
6161
collision: false,
6262
});
6363
});
6464

65-
test("rejects the exact same refresh grant under a different alias", async () => {
65+
test("rejects the same personal account within the personal bucket", async () => {
6666
seedAccount("team-member-a", "member-a@example.test", "shared-team-account");
6767

68-
const result = checkAccountIdCollision("different-account-id", "other@example.test", "refresh-team-member-a");
68+
const result = checkAccountIdCollision("shared-team-account", "MEMBER-A@example.test");
6969
expect(result.collision).toBe(true);
7070
if (result.collision) {
71-
expect(result.reason).toContain("Credential is already in the pool");
71+
expect(result.reason).toContain("Account is already in the pool");
72+
}
73+
});
74+
75+
test("rejects the same workspace account within the workspace bucket", async () => {
76+
seedAccount("workspace-a", "member-a@example.test", "shared-team-account", "team");
77+
78+
const result = checkAccountIdCollision("shared-team-account", "MEMBER-A@example.test", "business");
79+
expect(result.collision).toBe(true);
80+
if (result.collision) {
81+
expect(result.reason).toContain("Account is already in the pool");
7282
}
7383
});
7484
});

0 commit comments

Comments
 (0)