diff --git a/src/main/codex-accounts/legacy-shared-auth-migration.test.ts b/src/main/codex-accounts/legacy-shared-auth-migration.test.ts index f77e9c390a3..d7274c3e227 100644 --- a/src/main/codex-accounts/legacy-shared-auth-migration.test.ts +++ b/src/main/codex-accounts/legacy-shared-auth-migration.test.ts @@ -142,6 +142,93 @@ describe('legacy shared Codex auth migration', () => { }) }) +describe('legacy shared Codex MCP credentials migration (#8440)', () => { + it('carries the shared MCP .credentials.json into the identity-proven home with 0600 perms', () => { + const stale = createAuth('one@example.com', 'acct-1', 'stale', 1_000) + const fresh = createAuth('one@example.com', 'acct-1', 'fresh', 2_000) + const account = fixture.createAccount('account-1', 'acct-1', stale) + fixture.writeSharedAuth(fresh) + const mcpStore = JSON.stringify({ MCP_OAUTH: { 'server-a': { access_token: 'tok-a' } } }) + fixture.writeSharedCredentials(mcpStore) + + fixture.migrate([account], account.id) + + const credentialsPath = fixture.accountCredentialsPath(account) + expect(readFileSync(credentialsPath, 'utf-8')).toBe(mcpStore) + if (process.platform !== 'win32') { + expect(statSync(credentialsPath).mode & 0o777).toBe(0o600) + } + expect(fixture.marker()).toMatchObject({ outcome: 'migrated', accountId: account.id }) + }) + + it('carries the shared MCP store even when auth.json is already current', () => { + const fresh = createAuth('one@example.com', 'acct-1', 'fresh', 2_000) + const account = fixture.createAccount('account-1', 'acct-1', fresh) + fixture.writeSharedAuth(fresh) + const mcpStore = JSON.stringify({ MCP_OAUTH: { 'server-a': { access_token: 'tok-a' } } }) + fixture.writeSharedCredentials(mcpStore) + + fixture.migrate([account], account.id) + + expect(readFileSync(fixture.accountCredentialsPath(account), 'utf-8')).toBe(mcpStore) + expect(fixture.marker()).toMatchObject({ outcome: 'already-current' }) + }) + + it('is a no-op when the shared mirror has no MCP .credentials.json', () => { + const stale = createAuth('one@example.com', 'acct-1', 'stale', 1_000) + const fresh = createAuth('one@example.com', 'acct-1', 'fresh', 2_000) + const account = fixture.createAccount('account-1', 'acct-1', stale) + fixture.writeSharedAuth(fresh) + + fixture.migrate([account], account.id) + + expect(existsSync(fixture.accountCredentialsPath(account))).toBe(false) + expect(fixture.marker()).toMatchObject({ outcome: 'migrated' }) + }) + + it('never clobbers a newer per-account .credentials.json', () => { + const stale = createAuth('one@example.com', 'acct-1', 'stale', 1_000) + const fresh = createAuth('one@example.com', 'acct-1', 'fresh', 2_000) + const account = fixture.createAccount('account-1', 'acct-1', stale) + fixture.writeSharedAuth(fresh) + fixture.writeSharedCredentials( + JSON.stringify({ MCP_OAUTH: { 'server-a': { access_token: 'shared-stale' } } }) + ) + const perAccountStore = JSON.stringify({ + MCP_OAUTH: { 'server-a': { access_token: 'per-account-fresh' } } + }) + writeFileSync(fixture.accountCredentialsPath(account), perAccountStore, 'utf-8') + + fixture.migrate([account], account.id) + + expect(readFileSync(fixture.accountCredentialsPath(account), 'utf-8')).toBe(perAccountStore) + }) + + it('never leaks the shared MCP store to a non-matching account', () => { + const account1 = fixture.createAccount( + 'account-1', + 'acct-duplicate', + createAuth('same@example.com', 'acct-duplicate', 'one', 1_000) + ) + const account2 = fixture.createAccount( + 'account-2', + 'acct-duplicate', + createAuth('same@example.com', 'acct-duplicate', 'two', 1_000) + ) + fixture.writeSharedAuth(createAuth('same@example.com', 'acct-duplicate', 'shared', 2_000)) + fixture.writeSharedCredentials( + JSON.stringify({ MCP_OAUTH: { 'server-a': { access_token: 'tok-a' } } }) + ) + + // Ambiguous identity: neither account is a unique match, so nothing is carried. + fixture.migrate([account1, account2], account1.id) + + expect(existsSync(fixture.accountCredentialsPath(account1))).toBe(false) + expect(existsSync(fixture.accountCredentialsPath(account2))).toBe(false) + expect(existsSync(fixture.markerPath)).toBe(false) + }) +}) + function createFixture() { const root = mkdtempSync(join(tmpdir(), 'orca-codex-auth-migration-')) const managedAccountsRoot = join(root, 'codex-accounts') @@ -175,6 +262,12 @@ function createFixture() { writeSharedAuth(auth: string) { writeFileSync(join(sharedRuntimeHome, 'auth.json'), auth, 'utf-8') }, + writeSharedCredentials(credentials: string) { + writeFileSync(join(sharedRuntimeHome, '.credentials.json'), credentials, 'utf-8') + }, + accountCredentialsPath(account: CodexManagedAccount) { + return join(account.managedHomePath, '.credentials.json') + }, migrate(hostAccounts: readonly CodexManagedAccount[], activeHostAccountId: string | null) { migrateLegacySharedAuthToPerAccountHome({ activeHostAccountId, diff --git a/src/main/codex-accounts/legacy-shared-auth-migration.ts b/src/main/codex-accounts/legacy-shared-auth-migration.ts index 7a697e10a5e..d3a9f247ac4 100644 --- a/src/main/codex-accounts/legacy-shared-auth-migration.ts +++ b/src/main/codex-accounts/legacy-shared-auth-migration.ts @@ -20,6 +20,7 @@ type TrustedAccountAuth = { account: CodexManagedAccount authContents: string | null authPath: string + homePath: string } type CompletedOutcome = 'already-current' | 'migrated' | 'no-shared-auth' | 'not-newer' @@ -62,6 +63,14 @@ export function migrateLegacySharedAuthToPerAccountHome({ if (match.authContents === null) { return } + + // Why: Codex file-mode MCP OAuth tokens live in $CODEX_HOME/.credentials.json + // (issue #8440), keyed by MCP server URL with no account identity of their + // own. The uniquely-matched auth.json is the only proof this shared mirror is + // the active account's, so carry its co-located MCP store into the same + // per-account home or those servers silently need re-auth after upgrade. + migrateSharedMcpCredentials(sharedRuntimeHome, match.homePath) + if (match.authContents === sharedAuthContents) { writeCompletedMarker(markerPath, 'already-current', match.account.id) return @@ -93,7 +102,30 @@ function readTrustedAccountAuth( expectedAccountId: account.id }) const authPath = join(trustedHome, 'auth.json') - return { account, authContents: readRegularFile(authPath), authPath } + return { + account, + authContents: readRegularFile(authPath), + authPath, + homePath: trustedHome + } +} + +// Why: MCP OAuth tokens have no identity claim to match on, so we only ever +// write the shared store into the one identity-proven account's home, and never +// clobber a newer .credentials.json that account authed in its own home going +// forward. Absent source is a plain no-op. +function migrateSharedMcpCredentials(sharedRuntimeHome: string, perAccountHome: string): void { + const sharedCredentials = readRegularFile(join(sharedRuntimeHome, '.credentials.json')) + if (sharedCredentials === null) { + return + } + const perAccountCredentialsPath = join(perAccountHome, '.credentials.json') + if (regularFileState(perAccountCredentialsPath) === 'present') { + return + } + // Why: atomic 0600 write mirrors auth.json so a crash cannot leave a partial + // MCP store or widen permissions on sensitive tokens. + writeFileAtomically(perAccountCredentialsPath, sharedCredentials, { mode: 0o600 }) } function readRegularFile(filePath: string): string | null {