Merge pull request #194 from MiniMax-AI/codex/region-detection-fail-closed

fix: fail closed when API key region detection is inconclusive

Author: NianJiuZst

src/commands/auth/login.ts

@@ -20,6 +20,37 @@ interface QuotaApiResponse {
   model_remains: QuotaModelRemain[];
 }
 
+function isNetworkUnavailable(error: unknown): boolean {
+  if (error instanceof CLIError) {
+    return error.exitCode === ExitCode.NETWORK || error.exitCode === ExitCode.TIMEOUT;
+  }
+
+  if (!(error instanceof Error)) return false;
+
+  if (error.name === 'AbortError' || error.name === 'TimeoutError') return true;
+
+  const message = error.message.toLowerCase();
+  const code = 'code' in error ? String(error.code).toLowerCase() : '';
+  return (error instanceof TypeError && message === 'fetch failed')
+    || message.includes('failed to fetch')
+    || message.includes('unable to connect')
+    || message.includes('connection refused')
+    || message.includes('econnrefused')
+    || message.includes('connection reset')
+    || message.includes('econnreset')
+    || message.includes('network error')
+    || message.includes('enotfound')
+    || message.includes('getaddrinfo')
+    || message.includes('proxy')
+    || message.includes('socket')
+    || message.includes('etimedout')
+    || code === 'connectionrefused'
+    || code === 'econnrefused'
+    || code === 'econnreset'
+    || code === 'enotfound'
+    || code === 'etimedout';
+}
+
 async function showQuotaAfterLogin(config: Config): Promise<void> {
   try {
     const url = quotaEndpoint(config.baseUrl);
@@ -76,7 +107,7 @@ export default defineCommand({
     }
 
     if (flags.apiKey) {
-      await loginWithApiKey(config, flags.apiKey as string);
+      await loginWithApiKey(config, flags.apiKey as string, flags.region as Region | undefined);
       return;
     }
 
@@ -109,7 +140,7 @@ export default defineCommand({
         validate: (v) => (v && v.length > 0 ? undefined : 'API key cannot be empty.'),
       });
       if (isCancel(key)) throw new CLIError('Authentication cancelled.', ExitCode.AUTH);
-      await loginWithApiKey(config, key as string);
+      await loginWithApiKey(config, key as string, flags.region as Region | undefined);
       return;
     }
 
@@ -131,26 +162,41 @@ async function completeOAuthLogin(config: Config, region: Region): Promise<void>
   await showDashboardAfterLogin(cfg);
 }
 
-async function loginWithApiKey(config: Config, key: string): Promise<void> {
+async function loginWithApiKey(
+  config: Config,
+  key: string,
+  explicitRegion?: Region,
+): Promise<void> {
   if (config.dryRun) {
     console.log('Would validate and save API key.');
     return;
   }
 
-  // Probe both regions and pick the one the key actually authenticates against.
-  // This doubles as key validation — if neither region accepts it, the key is bad.
-  const detected = await detectRegion(key);
+  // An explicit region is authoritative. Otherwise probe both regions and fail
+  // closed if neither can be confirmed; never persist a guessed fallback.
+  const detected = explicitRegion ?? await detectRegion(key);
   const cfg: Config = { ...config, region: detected, baseUrl: REGIONS[detected], apiKey: key };
 
-  // Verify the detection actually authorizes the quota endpoint (defends against
-  // detectRegion's graceful 'global' fallback when the network is unreachable).
+  // Verify the selected region actually authorizes the quota endpoint.
   try {
     await requestJson<QuotaApiResponse>(cfg, { url: quotaEndpoint(cfg.baseUrl) });
-  } catch {
-    throw new CLIError(
-      'API key validation failed.',
-      ExitCode.AUTH,
-      'Check that your key is valid and belongs to a Token Plan.',
+  } catch (error) {
+    if (error instanceof CLIError && error.exitCode === ExitCode.AUTH) {
+      throw new CLIError(
+        'API key validation failed.',
+        ExitCode.AUTH,
+        'Check that your key is valid and belongs to a Token Plan.',
+      );
+    }
+
+    if (!explicitRegion) throw error;
+
+    const validationFailure = isNetworkUnavailable(error)
+      ? `the ${detected} API endpoint is unreachable`
+      : `the ${detected} API endpoint returned an inconclusive response`;
+    process.stderr.write(
+      `Warning: Could not validate the API key because ${validationFailure}.\n` +
+      `Saving the explicitly selected region "${detected}". Requests may fail until validation succeeds.\n`,
     );
   }
 

src/config/detect-region.ts

@@ -1,8 +1,12 @@
 import { REGIONS, type Region } from "./schema";
 import { readConfigFile, writeConfigFile } from "./loader";
+import { CLIError } from "../errors/base";
+import { ExitCode } from "../errors/codes";
 
 const QUOTA_PATH = "/v1/token_plan/remains";
 
+type ProbeResult = "authorized" | "unauthorized" | "unreachable" | "inconclusive";
+
 function quotaUrl(region: Region): string {
   return REGIONS[region] + QUOTA_PATH;
 }
@@ -11,7 +15,7 @@ async function probeRegion(
   region: Region,
   apiKey: string,
   timeoutMs: number,
-): Promise<boolean> {
+): Promise<ProbeResult> {
   // MiniMax endpoints accept either Bearer or x-api-key auth — try both.
   // Some API key types only work with one style; trying both prevents false
   // negatives that would cause the wrong region to be selected, leading to
@@ -21,22 +25,41 @@ async function probeRegion(
     { "x-api-key": apiKey },
   ];
 
+  let sawNetworkFailure = false;
+  let sawInconclusiveResponse = false;
+
   for (const authHeader of authHeaders) {
+    let res: Response;
     try {
-      const res = await fetch(quotaUrl(region), {
+      res = await fetch(quotaUrl(region), {
         headers: { ...authHeader, "Content-Type": "application/json" },
         signal: AbortSignal.timeout(timeoutMs),
       });
-      if (!res.ok) continue;
+    } catch {
+      sawNetworkFailure = true;
+      continue;
+    }
+
+    if (res.status === 401 || res.status === 403) continue;
+    if (!res.ok) {
+      sawInconclusiveResponse = true;
+      continue;
+    }
+
+    try {
       const data = (await res.json()) as {
         base_resp?: { status_code?: number };
       };
-      if (data.base_resp?.status_code === 0) return true;
+      if (data.base_resp?.status_code === 0) return "authorized";
+      sawInconclusiveResponse = true;
     } catch {
-      // Try next auth style before giving up on this region
+      sawInconclusiveResponse = true;
     }
   }
-  return false;
+
+  if (sawInconclusiveResponse) return "inconclusive";
+  if (sawNetworkFailure) return "unreachable";
+  return "unauthorized";
 }
 
 export async function detectRegion(apiKey: string): Promise<Region> {
@@ -45,19 +68,34 @@ export async function detectRegion(apiKey: string): Promise<Region> {
   const results = await Promise.all(
     regions.map(async (r) => ({
       region: r,
-      ok: await probeRegion(r, apiKey, 5000),
+      result: await probeRegion(r, apiKey, 5000),
     })),
   );
-  const match = results.find((r) => r.ok);
+  const match = results.find((r) => r.result === "authorized");
   if (!match) {
     process.stderr.write(" failed\n");
-    process.stderr.write(
-      `Warning: API key failed validation against all regions (global, cn).\n` +
-      `  This usually means the API key is invalid or the network is blocking requests.\n` +
-      `  Falling back to 'global'. Subsequent requests may fail.\n` +
-      `  Run 'mmx auth status' to verify your credentials.\n`,
+
+    if (results.every((r) => r.result === "unauthorized")) {
+      throw new CLIError(
+        "API key was rejected by all regions.",
+        ExitCode.AUTH,
+        "No credentials were changed. Check that the key is valid and belongs to a Token Plan.",
+      );
+    }
+
+    if (results.some((r) => r.result === "unreachable")) {
+      throw new CLIError(
+        "Could not reach the regional API endpoints.",
+        ExitCode.NETWORK,
+        "No region was changed. Check the network or proxy, then retry or pass --region global|cn explicitly.",
+      );
+    }
+
+    throw new CLIError(
+      "Could not determine the API key region.",
+      ExitCode.GENERAL,
+      "No region was changed because the API returned an inconclusive response. Retry later or pass --region global|cn explicitly.",
     );
-    return "global";
   }
   const detected: Region = match.region;
   process.stderr.write(` ${detected}\n`);

test/auth/timeout-fix.test.ts

@@ -77,7 +77,7 @@ describe('detect-region: probeRegion auth style fallback', () => {
     }
   });
 
-  it('falls back to global when key is invalid for all auth styles and regions', async () => {
+  it('fails closed when key is invalid for all auth styles and regions', async () => {
     server = createMockServer({
       routes: {
         '/v1/token_plan/remains': () =>
@@ -93,8 +93,37 @@ describe('detect-region: probeRegion auth style fallback', () => {
 
     try {
       const { detectRegion } = await import('../../src/config/detect-region');
-      const region = await detectRegion('bad-key');
-      expect(region).toBe('global'); // graceful fallback
+      try {
+        await detectRegion('bad-key');
+        throw new Error('Expected region detection to fail');
+      } catch (error) {
+        expect(error).toBeInstanceOf(CLIError);
+        expect((error as CLIError).message).toBe('API key was rejected by all regions.');
+        expect((error as CLIError).exitCode).toBe(ExitCode.AUTH);
+      }
+    } finally {
+      (REGIONS as Record<string, string>).global = origGlobal;
+      (REGIONS as Record<string, string>).cn = origCn;
+    }
+  });
+
+  it('reports a network error when no regional endpoint is reachable', async () => {
+    const { REGIONS } = await import('../../src/config/schema');
+    const origGlobal = REGIONS.global;
+    const origCn = REGIONS.cn;
+    (REGIONS as Record<string, string>).global = 'http://127.0.0.1:1';
+    (REGIONS as Record<string, string>).cn = 'http://127.0.0.1:1';
+
+    try {
+      const { detectRegion } = await import('../../src/config/detect-region');
+      try {
+        await detectRegion('unverifiable-key');
+        throw new Error('Expected region detection to fail');
+      } catch (error) {
+        expect(error).toBeInstanceOf(CLIError);
+        expect((error as CLIError).message).toBe('Could not reach the regional API endpoints.');
+        expect((error as CLIError).exitCode).toBe(ExitCode.NETWORK);
+      }
     } finally {
       (REGIONS as Record<string, string>).global = origGlobal;
       (REGIONS as Record<string, string>).cn = origCn;