Skip to content

Commit 98b9bec

Browse files
committed
fix: fail closed when region detection is inconclusive
1 parent eca6acf commit 98b9bec

4 files changed

Lines changed: 103 additions & 18 deletions

File tree

src/commands/auth/login.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export default defineCommand({
7676
}
7777

7878
if (flags.apiKey) {
79-
await loginWithApiKey(config, flags.apiKey as string);
79+
await loginWithApiKey(config, flags.apiKey as string, flags.region as Region | undefined);
8080
return;
8181
}
8282

@@ -109,7 +109,7 @@ export default defineCommand({
109109
validate: (v) => (v && v.length > 0 ? undefined : 'API key cannot be empty.'),
110110
});
111111
if (isCancel(key)) throw new CLIError('Authentication cancelled.', ExitCode.AUTH);
112-
await loginWithApiKey(config, key as string);
112+
await loginWithApiKey(config, key as string, flags.region as Region | undefined);
113113
return;
114114
}
115115

@@ -131,19 +131,22 @@ async function completeOAuthLogin(config: Config, region: Region): Promise<void>
131131
await showDashboardAfterLogin(cfg);
132132
}
133133

134-
async function loginWithApiKey(config: Config, key: string): Promise<void> {
134+
async function loginWithApiKey(
135+
config: Config,
136+
key: string,
137+
explicitRegion?: Region,
138+
): Promise<void> {
135139
if (config.dryRun) {
136140
console.log('Would validate and save API key.');
137141
return;
138142
}
139143

140-
// Probe both regions and pick the one the key actually authenticates against.
141-
// This doubles as key validation — if neither region accepts it, the key is bad.
142-
const detected = await detectRegion(key);
144+
// An explicit region is authoritative. Otherwise probe both regions and fail
145+
// closed if neither can be confirmed; never persist a guessed fallback.
146+
const detected = explicitRegion ?? await detectRegion(key);
143147
const cfg: Config = { ...config, region: detected, baseUrl: REGIONS[detected], apiKey: key };
144148

145-
// Verify the detection actually authorizes the quota endpoint (defends against
146-
// detectRegion's graceful 'global' fallback when the network is unreachable).
149+
// Verify the selected region actually authorizes the quota endpoint.
147150
try {
148151
await requestJson<QuotaApiResponse>(cfg, { url: quotaEndpoint(cfg.baseUrl) });
149152
} catch {

src/config/detect-region.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { REGIONS, type Region } from "./schema";
22
import { readConfigFile, writeConfigFile } from "./loader";
3+
import { CLIError } from "../errors/base";
4+
import { ExitCode } from "../errors/codes";
35

46
const QUOTA_PATH = "/v1/token_plan/remains";
57

@@ -51,13 +53,11 @@ export async function detectRegion(apiKey: string): Promise<Region> {
5153
const match = results.find((r) => r.ok);
5254
if (!match) {
5355
process.stderr.write(" failed\n");
54-
process.stderr.write(
55-
`Warning: API key failed validation against all regions (global, cn).\n` +
56-
` This usually means the API key is invalid or the network is blocking requests.\n` +
57-
` Falling back to 'global'. Subsequent requests may fail.\n` +
58-
` Run 'mmx auth status' to verify your credentials.\n`,
56+
throw new CLIError(
57+
"Could not determine the API key region.",
58+
ExitCode.AUTH,
59+
"No region was changed. Check the key and network, then retry or pass --region global|cn explicitly.",
5960
);
60-
return "global";
6161
}
6262
const detected: Region = match.region;
6363
process.stderr.write(` ${detected}\n`);

test/auth/timeout-fix.test.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe('detect-region: probeRegion auth style fallback', () => {
7777
}
7878
});
7979

80-
it('falls back to global when key is invalid for all auth styles and regions', async () => {
80+
it('fails closed when key is invalid for all auth styles and regions', async () => {
8181
server = createMockServer({
8282
routes: {
8383
'/v1/token_plan/remains': () =>
@@ -93,8 +93,14 @@ describe('detect-region: probeRegion auth style fallback', () => {
9393

9494
try {
9595
const { detectRegion } = await import('../../src/config/detect-region');
96-
const region = await detectRegion('bad-key');
97-
expect(region).toBe('global'); // graceful fallback
96+
try {
97+
await detectRegion('bad-key');
98+
throw new Error('Expected region detection to fail');
99+
} catch (error) {
100+
expect(error).toBeInstanceOf(CLIError);
101+
expect((error as CLIError).message).toBe('Could not determine the API key region.');
102+
expect((error as CLIError).exitCode).toBe(ExitCode.AUTH);
103+
}
98104
} finally {
99105
(REGIONS as Record<string, string>).global = origGlobal;
100106
(REGIONS as Record<string, string>).cn = origCn;

test/commands/auth/login.test.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,25 @@
1-
import { describe, it, expect } from 'bun:test';
1+
import { afterEach, describe, expect, it } from 'bun:test';
2+
import { mkdtempSync, rmSync } from 'node:fs';
3+
import { tmpdir } from 'node:os';
4+
import { join } from 'node:path';
25
import { default as loginCommand } from '../../../src/commands/auth/login';
6+
import { REGIONS } from '../../../src/config/schema';
7+
import { createMockServer, jsonResponse, type MockServer } from '../../helpers/mock-server';
38

49
describe('auth login command', () => {
10+
const originalConfigDir = process.env.MMX_CONFIG_DIR;
11+
let server: MockServer | undefined;
12+
let configDir: string | undefined;
13+
14+
afterEach(() => {
15+
server?.close();
16+
if (configDir) rmSync(configDir, { recursive: true, force: true });
17+
if (originalConfigDir === undefined) delete process.env.MMX_CONFIG_DIR;
18+
else process.env.MMX_CONFIG_DIR = originalConfigDir;
19+
server = undefined;
20+
configDir = undefined;
21+
});
22+
523
it('has correct name and description', () => {
624
expect(loginCommand.name).toBe('auth login');
725
expect(loginCommand.description).toContain('Authenticate');
@@ -35,4 +53,62 @@ describe('auth login command', () => {
3553
}),
3654
).rejects.toThrow('--api-key is required');
3755
});
56+
57+
it('honors an explicit region without running auto-detection', async () => {
58+
let quotaRequests = 0;
59+
server = createMockServer({
60+
routes: {
61+
'/v1/token_plan/remains': () => {
62+
quotaRequests += 1;
63+
return jsonResponse({
64+
model_remains: [],
65+
base_resp: { status_code: 0, status_msg: 'ok' },
66+
});
67+
},
68+
},
69+
});
70+
71+
configDir = mkdtempSync(join(tmpdir(), 'mmx-region-login-'));
72+
process.env.MMX_CONFIG_DIR = configDir;
73+
74+
const originalGlobal = REGIONS.global;
75+
const originalCn = REGIONS.cn;
76+
(REGIONS as Record<string, string>).global = 'http://127.0.0.1:1';
77+
(REGIONS as Record<string, string>).cn = server.url;
78+
79+
try {
80+
await loginCommand.execute(
81+
{
82+
region: 'global',
83+
baseUrl: originalGlobal,
84+
output: 'text',
85+
timeout: 1,
86+
verbose: false,
87+
quiet: true,
88+
noColor: true,
89+
yes: false,
90+
dryRun: false,
91+
nonInteractive: true,
92+
async: false,
93+
},
94+
{
95+
apiKey: 'cn-test-key',
96+
region: 'cn',
97+
quiet: true,
98+
verbose: false,
99+
noColor: true,
100+
yes: false,
101+
dryRun: false,
102+
help: false,
103+
nonInteractive: true,
104+
async: false,
105+
},
106+
);
107+
108+
expect(quotaRequests).toBe(2);
109+
} finally {
110+
(REGIONS as Record<string, string>).global = originalGlobal;
111+
(REGIONS as Record<string, string>).cn = originalCn;
112+
}
113+
});
38114
});

0 commit comments

Comments
 (0)