From 8651627842a427ac20ea3d83578bd38c5ea65b5f Mon Sep 17 00:00:00 2001 From: ding113 Date: Thu, 14 May 2026 23:35:39 +0800 Subject: [PATCH 1/6] fix(auth): return disabled/expired key errors instead of 429 lockout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disabled keys hit the proxy pre-auth rate limiter on every request, incremented the failure counter, and locked the IP/key out with HTTP 429 "Too many authentication failures" after 20 attempts. The legitimate "key disabled" 401 was masked forever once the lockout tripped. Introduce `resolveApiKeyAuthOutcome` returning a discriminated union (`not_found` / `key_disabled` / `key_expired` / ok), map each reason to its own 401 error, and only feed `credentials` failures to the rate-limiter — admin-disabled or expired keys/users now bypass it entirely. `validateApiKeyAndGetUser` stays as a backwards-compatible wrapper. The /v1/models chain is updated the same way. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/v1/_lib/models/available-models.ts | 15 +- src/app/v1/_lib/proxy/auth-guard.ts | 78 ++++-- src/app/v1/_lib/proxy/session.ts | 19 ++ src/repository/index.ts | 2 + src/repository/key.ts | 77 ++++-- .../available-models-gemini-key.test.ts | 5 + .../proxy/auth-guard-account-state.test.ts | 213 ++++++++++++++++ tests/unit/proxy/auth-guard-precheck.test.ts | 15 +- tests/unit/proxy/available-models.test.ts | 1 + .../repository/key-resolve-outcome.test.ts | 239 ++++++++++++++++++ 10 files changed, 618 insertions(+), 46 deletions(-) create mode 100644 tests/unit/proxy/auth-guard-account-state.test.ts create mode 100644 tests/unit/repository/key-resolve-outcome.test.ts diff --git a/src/app/v1/_lib/models/available-models.ts b/src/app/v1/_lib/models/available-models.ts index 7361f9db0..0538ec225 100644 --- a/src/app/v1/_lib/models/available-models.ts +++ b/src/app/v1/_lib/models/available-models.ts @@ -5,7 +5,7 @@ import { logger } from "@/lib/logger"; import { createProxyAgentForProvider } from "@/lib/proxy-agent"; import { isProviderActiveNow } from "@/lib/utils/provider-schedule"; import { resolveSystemTimezone } from "@/lib/utils/timezone"; -import { validateApiKeyAndGetUser } from "@/repository/key"; +import { resolveApiKeyAuthOutcome } from "@/repository/key"; import { findAllProviders } from "@/repository/provider"; import type { AnthropicModelsResponse, @@ -60,12 +60,19 @@ async function authenticateRequest(c: Context): Promise<{ throw c.json({ error: { message: "未提供认证凭据", type: "authentication_error" } }, 401); } - const authResult = await validateApiKeyAndGetUser(apiKey); - if (!authResult) { + const outcome = await resolveApiKeyAuthOutcome(apiKey); + if (!outcome.ok) { + if (outcome.reason === "key_disabled") { + throw c.json({ error: { message: "API 密钥已被禁用", type: "key_disabled" } }, 401); + } + if (outcome.reason === "key_expired") { + throw c.json({ error: { message: "API 密钥已过期", type: "key_expired" } }, 401); + } + // not_found throw c.json({ error: { message: "API 密钥无效", type: "invalid_api_key" } }, 401); } - const { user, key } = authResult; + const { user, key } = outcome; if (!user.isEnabled) { throw c.json({ error: { message: "用户账户已被禁用", type: "user_disabled" } }, 401); diff --git a/src/app/v1/_lib/proxy/auth-guard.ts b/src/app/v1/_lib/proxy/auth-guard.ts index 061e80404..27eb7433b 100644 --- a/src/app/v1/_lib/proxy/auth-guard.ts +++ b/src/app/v1/_lib/proxy/auth-guard.ts @@ -2,7 +2,7 @@ import { extractApiKeyFromHeaders as sharedExtractApiKeyFromHeaders } from "@/li import { getClientIpWithFreshSettings } from "@/lib/ip"; import { logger } from "@/lib/logger"; import { LoginAbusePolicy } from "@/lib/security/login-abuse-policy"; -import { validateApiKeyAndGetUser } from "@/repository/key"; +import { resolveApiKeyAuthOutcome } from "@/repository/key"; import { markUserExpired } from "@/repository/user"; import { GEMINI_PROTOCOL } from "../gemini/protocol"; import { ProxyResponses } from "./responses"; @@ -73,8 +73,13 @@ export class ProxyAuthenticator { return null; } - // Record failure for rate limiting - proxyAuthPolicy.recordFailure(clientIp, authState.apiKey ?? candidateApiKey ?? undefined); + // Only `credentials` failures should feed the brute-force rate limiter. + // `account_state` failures (key/user disabled or expired) match a real + // record, so recording them would let an admin lock themselves out by + // simply disabling a key and watching the owner retry. + if (authState.failureKind !== "account_state") { + proxyAuthPolicy.recordFailure(clientIp, authState.apiKey ?? candidateApiKey ?? undefined); + } // 返回详细的错误信息，帮助用户快速定位问题 return authState.errorResponse ?? ProxyResponses.buildError(401, "认证失败"); @@ -133,6 +138,7 @@ export class ProxyAuthenticator { key: null, apiKey: null, success: false, + failureKind: "credentials", errorResponse: ProxyResponses.buildError( 401, "未提供认证凭据。请在 Authorization 头部、x-api-key 头部或 x-goog-api-key 头部中包含 API 密钥。", @@ -153,6 +159,7 @@ export class ProxyAuthenticator { key: null, apiKey: null, success: false, + failureKind: "credentials", errorResponse: ProxyResponses.buildError( 401, "提供了多个冲突的 API 密钥。请仅使用一种认证方式。", @@ -162,29 +169,68 @@ export class ProxyAuthenticator { } const apiKey = firstKey; - const authResult = await validateApiKeyAndGetUser(apiKey); + const outcome = await resolveApiKeyAuthOutcome(apiKey); + + if (!outcome.ok) { + if (outcome.reason === "not_found") { + logger.debug("[ProxyAuthenticator] API key validation failed: not found", { + apiKeyLength: apiKey.length, + fromHeader: + !!headers.authHeader || !!headers.apiKeyHeader || !!headers.geminiApiKeyHeader, + fromQuery: !!headers.geminiApiKeyQuery, + }); + return { + user: null, + key: null, + apiKey, + success: false, + failureKind: "credentials", + errorResponse: ProxyResponses.buildError( + 401, + "API 密钥无效。提供的密钥不存在或已被删除。", + "invalid_api_key" + ), + }; + } + + if (outcome.reason === "key_disabled") { + logger.warn("[ProxyAuthenticator] API key is disabled", { + apiKeyLength: apiKey.length, + }); + return { + user: null, + key: null, + apiKey, + success: false, + failureKind: "account_state", + errorResponse: ProxyResponses.buildError( + 401, + "API 密钥已被禁用。请联系管理员重新启用，或使用其他可用密钥。", + "key_disabled" + ), + }; + } - if (!authResult) { - logger.debug("[ProxyAuthenticator] API key validation failed", { + // outcome.reason === "key_expired" + logger.warn("[ProxyAuthenticator] API key has expired", { apiKeyLength: apiKey.length, - fromHeader: !!headers.authHeader || !!headers.apiKeyHeader || !!headers.geminiApiKeyHeader, - fromQuery: !!headers.geminiApiKeyQuery, }); return { user: null, key: null, apiKey, success: false, + failureKind: "account_state", errorResponse: ProxyResponses.buildError( 401, - "API 密钥无效。提供的密钥不存在、已被删除、已被禁用或已过期。", - "invalid_api_key" + "API 密钥已过期。请联系管理员续期或更换密钥。", + "key_expired" ), }; } // Check user status and expiration - const { user } = authResult; + const { user } = outcome; // 1. Check if user is disabled if (!user.isEnabled) { @@ -197,6 +243,7 @@ export class ProxyAuthenticator { key: null, apiKey, success: false, + failureKind: "account_state", errorResponse: ProxyResponses.buildError( 401, "用户账户已被禁用。请联系管理员。", @@ -224,6 +271,7 @@ export class ProxyAuthenticator { key: null, apiKey, success: false, + failureKind: "account_state", errorResponse: ProxyResponses.buildError( 401, `用户账户已于 ${user.expiresAt.toISOString().split("T")[0]} 过期。请续费订阅。`, @@ -233,12 +281,12 @@ export class ProxyAuthenticator { } logger.debug("[ProxyAuthenticator] Authentication successful", { - userId: authResult.user.id, - userName: authResult.user.name, - keyName: authResult.key.name, + userId: outcome.user.id, + userName: outcome.user.name, + keyName: outcome.key.name, }); - return { user: authResult.user, key: authResult.key, apiKey, success: true }; + return { user: outcome.user, key: outcome.key, apiKey, success: true }; } private static extractKeyFromAuthorization(authHeader?: string): string | null { diff --git a/src/app/v1/_lib/proxy/session.ts b/src/app/v1/_lib/proxy/session.ts index dc2578ce3..b6dfda24e 100644 --- a/src/app/v1/_lib/proxy/session.ts +++ b/src/app/v1/_lib/proxy/session.ts @@ -30,12 +30,31 @@ import { parseOpenAIImageMultipartMetadata, } from "./openai-image-compat"; +/** + * Classification of an auth failure, used to decide whether to record the + * failure against the brute-force rate limiter. + * + * - `credentials`: the request did not present a valid key (missing, + * malformed, multiple conflicting keys, or the key does not match any + * record). These look like brute-force probes — record the failure. + * - `account_state`: the credentials matched a real record but the + * key/user is disabled, expired, or otherwise administratively rejected. + * Recording these as failures would lock out legitimate operators whose + * keys were disabled by an admin. + */ +export type AuthFailureKind = "credentials" | "account_state"; + export interface AuthState { user: User | null; key: Key | null; apiKey: string | null; success: boolean; errorResponse?: Response; // 认证失败时的详细错误响应 + /** + * Set when `success` is false. Determines whether the proxy auth guard + * records the failure against the IP/key rate-limiter. + */ + failureKind?: AuthFailureKind; } export interface MessageContext { diff --git a/src/repository/index.ts b/src/repository/index.ts index 299768236..3a41b23d0 100644 --- a/src/repository/index.ts +++ b/src/repository/index.ts @@ -5,6 +5,7 @@ import "server-only"; * 提供所有数据访问接口的统一入口 */ +export type { ApiKeyAuthFailureReason, ApiKeyAuthOutcome } from "./key"; // Key related exports export { countActiveKeysByUser, @@ -18,6 +19,7 @@ export { findKeysWithStatisticsBatch, findKeyUsageToday, findKeyUsageTodayBatch, + resolveApiKeyAuthOutcome, updateKey, validateApiKeyAndGetUser, } from "./key"; diff --git a/src/repository/key.ts b/src/repository/key.ts index 8cd19c788..e0ee26384 100644 --- a/src/repository/key.ts +++ b/src/repository/key.ts @@ -541,13 +541,35 @@ export async function findActiveKeyByKeyString(keyString: string): Promise { +/** + * Failure reasons surfaced by {@link resolveApiKeyAuthOutcome}. + * + * - `not_found`: key string does not exist (or matches a soft-deleted row, or + * the owning user was soft-deleted). Treated as a potential brute-force + * signal by the proxy auth guard. + * - `key_disabled` / `key_expired`: the key exists and the requester proved + * knowledge of it, but the key itself is no longer valid. These are NOT + * brute-force signals — they should return a specific error to the caller + * without incrementing the auth-failure rate limiter. + */ +export type ApiKeyAuthFailureReason = "not_found" | "key_disabled" | "key_expired"; + +export type ApiKeyAuthOutcome = + | { ok: true; user: User; key: Key } + | { ok: false; reason: ApiKeyAuthFailureReason }; + +/** + * Look up an API key and report a specific outcome so callers can distinguish + * "key never existed" from "key exists but disabled/expired". User-level + * status (disabled / expired) is intentionally NOT folded into this result — + * the caller inspects `user.isEnabled` / `user.expiresAt` directly so it can + * apply consistent semantics across the proxy and UI auth paths. + */ +export async function resolveApiKeyAuthOutcome(keyString: string): Promise { const vfSaysMissing = apiKeyVacuumFilter.isDefinitelyNotPresent(keyString) === true; // 默认鉴权链路：Vacuum Filter -> Redis -> DB + // Redis 缓存只保存活跃 key，命中即代表 ok=true。 const cachedKey = await getCachedActiveKey(keyString); if (cachedKey) { // 多实例一致性：若 Vacuum Filter 判定缺失但 Redis 命中，说明本机 filter 可能滞后。 @@ -558,7 +580,7 @@ export async function validateApiKeyAndGetUser( const cachedUser = await getCachedUser(cachedKey.userId); if (cachedUser) { - return { user: cachedUser, key: cachedKey }; + return { ok: true, user: cachedUser, key: cachedKey }; } // user 缓存 miss：仅补齐 user（相较 join 更轻量） @@ -596,20 +618,22 @@ export async function validateApiKeyAndGetUser( if (!userRow) { // join 语义：用户被删除则 key 无效；顺带清理 key 缓存避免重复 miss invalidateCachedKey(keyString).catch(() => {}); - return null; + return { ok: false, reason: "not_found" }; } const user = toUser(userRow); cacheUser(user).catch(() => {}); - return { user, key: cachedKey }; + return { ok: true, user, key: cachedKey }; } - // Vacuum Filter 负向短路：肯定不存在则直接返回 null，避免打 DB + // Vacuum Filter 负向短路：肯定不存在则直接返回 not_found，避免打 DB // 注意：此处必须放在 Redis 读取之后，避免多实例环境中新建 key 的短暂误拒绝窗口。 if (vfSaysMissing) { - return null; + return { ok: false, reason: "not_found" }; } + // 注意：此处放宽 WHERE 条件，不再过滤 isEnabled / expiresAt，由后续逻辑分类失败原因。 + // 用户软删除仍直接折叠成 not_found（用户不存在的语义与 key 不存在等价）。 const result = await db .select({ // Key fields @@ -663,22 +687,22 @@ export async function validateApiKeyAndGetUser( }) .from(keys) .innerJoin(users, eq(keys.userId, users.id)) - .where( - and( - eq(keys.key, keyString), - isNull(keys.deletedAt), - eq(keys.isEnabled, true), - or(isNull(keys.expiresAt), gt(keys.expiresAt, new Date())), - isNull(users.deletedAt) - ) - ); + .where(and(eq(keys.key, keyString), isNull(keys.deletedAt), isNull(users.deletedAt))); if (result.length === 0) { - return null; + return { ok: false, reason: "not_found" }; } const row = result[0]; + // 区分失败原因：先 disabled，再 expired，再 ok。 + if (row.keyIsEnabled !== true) { + return { ok: false, reason: "key_disabled" }; + } + if (row.keyExpiresAt && row.keyExpiresAt.getTime() <= Date.now()) { + return { ok: false, reason: "key_expired" }; + } + const user: User = toUser({ id: row.userId, name: row.userName, @@ -733,7 +757,20 @@ export async function validateApiKeyAndGetUser( // 最佳努力：写入 Redis 缓存（不影响正确性） cacheAuthResult(keyString, { user, key }).catch(() => {}); - return { user, key }; + return { ok: true, user, key }; +} + +/** + * Backwards-compatible wrapper around {@link resolveApiKeyAuthOutcome}: returns + * `null` on any lookup failure. Callers that need to distinguish failure + * reasons (e.g. the proxy auth guard) should call `resolveApiKeyAuthOutcome` + * directly. + */ +export async function validateApiKeyAndGetUser( + keyString: string +): Promise<{ user: User; key: Key } | null> { + const outcome = await resolveApiKeyAuthOutcome(keyString); + return outcome.ok ? { user: outcome.user, key: outcome.key } : null; } /** diff --git a/tests/unit/models/available-models-gemini-key.test.ts b/tests/unit/models/available-models-gemini-key.test.ts index 640a930e5..22421d7cb 100644 --- a/tests/unit/models/available-models-gemini-key.test.ts +++ b/tests/unit/models/available-models-gemini-key.test.ts @@ -25,6 +25,11 @@ vi.mock("@/repository/key", () => { user: { id: 1, providerGroup: null, isEnabled: true, expiresAt: null }, key: { providerGroup: null, name: "test-key" }, })), + resolveApiKeyAuthOutcome: vi.fn(async () => ({ + ok: true, + user: { id: 1, providerGroup: null, isEnabled: true, expiresAt: null }, + key: { providerGroup: null, name: "test-key" }, + })), }; }); diff --git a/tests/unit/proxy/auth-guard-account-state.test.ts b/tests/unit/proxy/auth-guard-account-state.test.ts new file mode 100644 index 000000000..2a3c56c1a --- /dev/null +++ b/tests/unit/proxy/auth-guard-account-state.test.ts @@ -0,0 +1,213 @@ +/** + * Regression coverage for the auth guard's handling of account-state failures: + * key disabled, key expired, user disabled, user expired. + * + * Each scenario must: + * - return HTTP 401 (NOT 429 — these are not rate-limit violations) + * - return a distinct, machine-readable error type + * - NOT call `recordFailure` on the brute-force rate limiter, since admin + * actions should not lock out the legitimate operator behind the key + */ + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const resolveApiKeyAuthOutcome = vi.hoisted(() => vi.fn()); +const policyCheck = vi.hoisted(() => vi.fn()); +const policyRecordSuccess = vi.hoisted(() => vi.fn()); +const policyRecordFailure = vi.hoisted(() => vi.fn()); +const markUserExpired = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); + +vi.mock("@/repository/key", () => ({ + resolveApiKeyAuthOutcome, +})); + +vi.mock("@/repository/user", () => ({ + markUserExpired, +})); + +vi.mock("@/lib/logger", () => ({ + logger: { + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +vi.mock("@/lib/security/login-abuse-policy", () => ({ + LoginAbusePolicy: class { + check = policyCheck; + recordSuccess = policyRecordSuccess; + recordFailure = policyRecordFailure; + }, +})); + +function makeSession(ip: string, apiKey: string) { + return { + headers: new Headers({ + "x-real-ip": ip, + "x-api-key": apiKey, + }), + requestUrl: new URL("http://localhost/v1/messages"), + clientIp: null as string | null, + authState: null as unknown, + setAuthState(state: unknown) { + this.authState = state; + }, + }; +} + +async function readErrorBody(response: Response) { + const json = (await response.clone().json()) as { + error: { message: string; type: string; code: string }; + }; + return json.error; +} + +describe("ProxyAuthenticator account-state failures", () => { + beforeEach(() => { + vi.resetModules(); + resolveApiKeyAuthOutcome.mockReset(); + policyCheck.mockReset().mockReturnValue({ allowed: true }); + policyRecordSuccess.mockReset(); + policyRecordFailure.mockReset(); + // vitest config sets mockReset: true globally, which wipes the + // mockResolvedValue from the hoisted setup. Re-apply per-test. + markUserExpired.mockReset().mockResolvedValue(undefined); + }); + + it("disabled key returns 401 key_disabled and does NOT count toward rate limit", async () => { + resolveApiKeyAuthOutcome.mockResolvedValue({ ok: false, reason: "key_disabled" }); + + const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); + const session = makeSession("203.0.113.20", "sk-disabled"); + const response = await ProxyAuthenticator.ensure(session as never); + + expect(response).not.toBeNull(); + expect(response?.status).toBe(401); + + const error = await readErrorBody(response as Response); + expect(error.type).toBe("key_disabled"); + expect(error.code).toBe("key_disabled"); + expect(error.message).toMatch(/已被禁用/); + + expect(policyRecordFailure).not.toHaveBeenCalled(); + expect(policyRecordSuccess).not.toHaveBeenCalled(); + }); + + it("expired key returns 401 key_expired and does NOT count toward rate limit", async () => { + resolveApiKeyAuthOutcome.mockResolvedValue({ ok: false, reason: "key_expired" }); + + const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); + const session = makeSession("203.0.113.21", "sk-expired"); + const response = await ProxyAuthenticator.ensure(session as never); + + expect(response?.status).toBe(401); + + const error = await readErrorBody(response as Response); + expect(error.type).toBe("key_expired"); + expect(error.code).toBe("key_expired"); + expect(error.message).toMatch(/已过期/); + + expect(policyRecordFailure).not.toHaveBeenCalled(); + }); + + it("disabled user returns 401 user_disabled and does NOT count toward rate limit", async () => { + resolveApiKeyAuthOutcome.mockResolvedValue({ + ok: true, + user: { id: 7, name: "bob", isEnabled: false, expiresAt: null }, + key: { name: "bobs-key" }, + }); + + const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); + const session = makeSession("203.0.113.22", "sk-userdisabled"); + const response = await ProxyAuthenticator.ensure(session as never); + + expect(response?.status).toBe(401); + + const error = await readErrorBody(response as Response); + expect(error.type).toBe("user_disabled"); + expect(error.message).toMatch(/账户已被禁用/); + + expect(policyRecordFailure).not.toHaveBeenCalled(); + }); + + it("expired user returns 401 user_expired, marks the user expired, and does NOT count toward rate limit", async () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000); + resolveApiKeyAuthOutcome.mockResolvedValue({ + ok: true, + user: { id: 8, name: "carol", isEnabled: true, expiresAt: yesterday }, + key: { name: "carols-key" }, + }); + + const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); + const session = makeSession("203.0.113.23", "sk-userexpired"); + const response = await ProxyAuthenticator.ensure(session as never); + + expect(response?.status).toBe(401); + + const error = await readErrorBody(response as Response); + expect(error.type).toBe("user_expired"); + expect(error.message).toMatch(/已于.*过期/); + + expect(markUserExpired).toHaveBeenCalledWith(8); + expect(policyRecordFailure).not.toHaveBeenCalled(); + }); + + it("unknown key still records failure (genuine brute-force signal)", async () => { + resolveApiKeyAuthOutcome.mockResolvedValue({ ok: false, reason: "not_found" }); + + const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); + const session = makeSession("203.0.113.24", "sk-doesnotexist"); + const response = await ProxyAuthenticator.ensure(session as never); + + expect(response?.status).toBe(401); + + const error = await readErrorBody(response as Response); + expect(error.type).toBe("invalid_api_key"); + expect(error.message).toMatch(/不存在|已被删除/); + + expect(policyRecordFailure).toHaveBeenCalledWith("203.0.113.24", "sk-doesnotexist"); + }); + + it("missing credentials returns 401 authentication_error and records failure", async () => { + const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); + const session = { + headers: new Headers({ "x-real-ip": "203.0.113.25" }), + requestUrl: new URL("http://localhost/v1/messages"), + clientIp: null as string | null, + authState: null as unknown, + setAuthState(state: unknown) { + this.authState = state; + }, + }; + const response = await ProxyAuthenticator.ensure(session as never); + + expect(response?.status).toBe(401); + + const error = await readErrorBody(response as Response); + expect(error.type).toBe("authentication_error"); + + expect(resolveApiKeyAuthOutcome).not.toHaveBeenCalled(); + expect(policyRecordFailure).toHaveBeenCalledWith("203.0.113.25", undefined); + }); + + it("repeated disabled-key attempts never trip the 429 lockout", async () => { + // Simulate the bug scenario: the same disabled key is hit 25 times in a + // row. Before the fix, the 20th attempt would trip the rate limiter and + // start returning 429s. After the fix, every attempt should return 401 + // key_disabled and the rate-limiter counter must remain untouched. + resolveApiKeyAuthOutcome.mockResolvedValue({ ok: false, reason: "key_disabled" }); + + const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); + + for (let attempt = 0; attempt < 25; attempt++) { + const session = makeSession("203.0.113.30", "sk-perma-disabled"); + const response = await ProxyAuthenticator.ensure(session as never); + expect(response?.status).toBe(401); + const error = await readErrorBody(response as Response); + expect(error.type).toBe("key_disabled"); + } + + expect(policyRecordFailure).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/proxy/auth-guard-precheck.test.ts b/tests/unit/proxy/auth-guard-precheck.test.ts index 66a526f84..f9b938a60 100644 --- a/tests/unit/proxy/auth-guard-precheck.test.ts +++ b/tests/unit/proxy/auth-guard-precheck.test.ts @@ -1,12 +1,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -const validateApiKeyAndGetUser = vi.hoisted(() => vi.fn()); +const resolveApiKeyAuthOutcome = vi.hoisted(() => vi.fn()); const policyCheck = vi.hoisted(() => vi.fn()); const policyRecordSuccess = vi.hoisted(() => vi.fn()); const policyRecordFailure = vi.hoisted(() => vi.fn()); vi.mock("@/repository/key", () => ({ - validateApiKeyAndGetUser, + resolveApiKeyAuthOutcome, })); vi.mock("@/repository/user", () => ({ @@ -47,7 +47,7 @@ function makeSession(ip: string, apiKey: string) { describe("ProxyAuthenticator pre-auth candidate key lockout", () => { beforeEach(() => { vi.resetModules(); - validateApiKeyAndGetUser.mockReset(); + resolveApiKeyAuthOutcome.mockReset(); policyCheck.mockReset(); policyRecordSuccess.mockReset(); policyRecordFailure.mockReset(); @@ -65,14 +65,15 @@ describe("ProxyAuthenticator pre-auth candidate key lockout", () => { const response = await ProxyAuthenticator.ensure(session as never); expect(response?.status).toBe(429); - expect(validateApiKeyAndGetUser).not.toHaveBeenCalled(); + expect(resolveApiKeyAuthOutcome).not.toHaveBeenCalled(); expect(policyCheck).toHaveBeenCalledWith("198.51.100.77", "sk-shared"); expect(session.clientIp).toBe("198.51.100.77"); }); it("resets both IP and key scopes on successful authentication", async () => { policyCheck.mockReturnValue({ allowed: true }); - validateApiKeyAndGetUser.mockResolvedValue({ + resolveApiKeyAuthOutcome.mockResolvedValue({ + ok: true, user: { id: 1, name: "alice", isEnabled: true, expiresAt: null }, key: { name: "primary-key" }, }); @@ -86,9 +87,9 @@ describe("ProxyAuthenticator pre-auth candidate key lockout", () => { expect(policyRecordFailure).not.toHaveBeenCalled(); }); - it("records failures against both IP and candidate key", async () => { + it("records failures against both IP and candidate key for unknown keys", async () => { policyCheck.mockReturnValue({ allowed: true }); - validateApiKeyAndGetUser.mockResolvedValue(null); + resolveApiKeyAuthOutcome.mockResolvedValue({ ok: false, reason: "not_found" }); const { ProxyAuthenticator } = await import("@/app/v1/_lib/proxy/auth-guard"); const session = makeSession("203.0.113.11", "sk-invalid"); diff --git a/tests/unit/proxy/available-models.test.ts b/tests/unit/proxy/available-models.test.ts index 51abfc7ac..86ce62ab2 100644 --- a/tests/unit/proxy/available-models.test.ts +++ b/tests/unit/proxy/available-models.test.ts @@ -7,6 +7,7 @@ vi.mock("@/lib/proxy-agent", () => ({ vi.mock("@/repository/key", () => ({ validateApiKeyAndGetUser: vi.fn(), + resolveApiKeyAuthOutcome: vi.fn(), })); vi.mock("@/app/v1/_lib/proxy/provider-selector", () => ({ diff --git a/tests/unit/repository/key-resolve-outcome.test.ts b/tests/unit/repository/key-resolve-outcome.test.ts new file mode 100644 index 000000000..b8d2d23ab --- /dev/null +++ b/tests/unit/repository/key-resolve-outcome.test.ts @@ -0,0 +1,239 @@ +/** + * Regression coverage for `resolveApiKeyAuthOutcome`: the discriminated union + * lookup must distinguish "key never existed" from "key exists but + * disabled/expired" so the proxy auth guard can return precise errors and + * skip the brute-force rate limiter for legitimate-but-rejected lookups. + */ + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const whereMock = vi.fn(); +const innerJoinMock = vi.fn(() => ({ where: whereMock })); +const fromMock = vi.fn(() => ({ innerJoin: innerJoinMock })); +const selectMock = vi.fn(() => ({ from: fromMock })); + +vi.mock("@/drizzle/db", () => ({ + db: { select: selectMock }, +})); + +vi.mock("@/drizzle/schema", () => ({ + keys: { + id: "keys.id", + userId: "keys.userId", + key: "keys.key", + name: "keys.name", + isEnabled: "keys.isEnabled", + expiresAt: "keys.expiresAt", + canLoginWebUi: "keys.canLoginWebUi", + limit5hUsd: "keys.limit5hUsd", + limitDailyUsd: "keys.limitDailyUsd", + dailyResetMode: "keys.dailyResetMode", + dailyResetTime: "keys.dailyResetTime", + limitWeeklyUsd: "keys.limitWeeklyUsd", + limitMonthlyUsd: "keys.limitMonthlyUsd", + limitTotalUsd: "keys.limitTotalUsd", + limitConcurrentSessions: "keys.limitConcurrentSessions", + providerGroup: "keys.providerGroup", + cacheTtlPreference: "keys.cacheTtlPreference", + createdAt: "keys.createdAt", + updatedAt: "keys.updatedAt", + deletedAt: "keys.deletedAt", + }, + users: { + id: "users.id", + name: "users.name", + description: "users.description", + role: "users.role", + rpmLimit: "users.rpmLimit", + dailyLimitUsd: "users.dailyLimitUsd", + providerGroup: "users.providerGroup", + limit5hUsd: "users.limit5hUsd", + limit5hResetMode: "users.limit5hResetMode", + limitWeeklyUsd: "users.limitWeeklyUsd", + limitMonthlyUsd: "users.limitMonthlyUsd", + limitTotalUsd: "users.limitTotalUsd", + costResetAt: "users.costResetAt", + limit5hCostResetAt: "users.limit5hCostResetAt", + limitConcurrentSessions: "users.limitConcurrentSessions", + dailyResetMode: "users.dailyResetMode", + dailyResetTime: "users.dailyResetTime", + isEnabled: "users.isEnabled", + expiresAt: "users.expiresAt", + allowedClients: "users.allowedClients", + allowedModels: "users.allowedModels", + createdAt: "users.createdAt", + updatedAt: "users.updatedAt", + deletedAt: "users.deletedAt", + }, + messageRequest: { + blockedBy: "messageRequest.blockedBy", + }, + usageLedger: { + blockedBy: "usageLedger.blockedBy", + endpoint: "usageLedger.endpoint", + }, + providers: { + id: "providers.id", + }, +})); + +vi.mock("drizzle-orm", () => { + const sqlMock = (...args: unknown[]) => args; + sqlMock.join = (...args: unknown[]) => args; + return { + and: (...args: unknown[]) => args, + or: (...args: unknown[]) => args, + eq: (...args: unknown[]) => args, + gt: (...args: unknown[]) => args, + gte: (...args: unknown[]) => args, + lt: (...args: unknown[]) => args, + isNull: (...args: unknown[]) => args, + count: (...args: unknown[]) => args, + desc: (...args: unknown[]) => args, + inArray: (...args: unknown[]) => args, + sql: sqlMock, + sum: (...args: unknown[]) => args, + }; +}); + +vi.mock("@/lib/security/api-key-auth-cache", () => ({ + getCachedActiveKey: vi.fn().mockResolvedValue(null), + getCachedUser: vi.fn().mockResolvedValue(null), + cacheActiveKey: vi.fn().mockResolvedValue(undefined), + cacheAuthResult: vi.fn().mockResolvedValue(undefined), + cacheUser: vi.fn().mockResolvedValue(undefined), + invalidateCachedKey: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/security/api-key-vacuum-filter", () => ({ + apiKeyVacuumFilter: { + isDefinitelyNotPresent: vi.fn().mockReturnValue(undefined), + noteExistingKey: vi.fn(), + }, +})); + +vi.mock("@/lib/redis/pubsub", () => ({ + CHANNEL_API_KEYS_UPDATED: "channel", + publishCacheInvalidation: vi.fn(), +})); + +function activeRow(overrides: Partial> = {}) { + const base = { + keyId: 1, + keyUserId: 2, + keyString: "sk-test", + keyName: "k1", + keyIsEnabled: true, + keyExpiresAt: null, + keyCanLoginWebUi: true, + keyLimit5hUsd: null, + keyLimit5hResetMode: "rolling", + keyLimitDailyUsd: null, + keyDailyResetMode: "fixed", + keyDailyResetTime: "00:00", + keyLimitWeeklyUsd: null, + keyLimitMonthlyUsd: null, + keyLimitTotalUsd: null, + keyCostResetAt: null, + keyLimitConcurrentSessions: 0, + keyProviderGroup: null, + keyCacheTtlPreference: null, + keyCreatedAt: new Date("2024-01-01T00:00:00.000Z"), + keyUpdatedAt: new Date("2024-01-01T00:00:00.000Z"), + keyDeletedAt: null, + userId: 2, + userName: "u1", + userDescription: "", + userRole: "user", + userRpm: null, + userDailyQuota: null, + userProviderGroup: null, + userLimit5hUsd: null, + userLimit5hResetMode: "rolling", + userLimitWeeklyUsd: null, + userLimitMonthlyUsd: null, + userLimitTotalUsd: null, + userCostResetAt: null, + userLimit5hCostResetAt: null, + userLimitConcurrentSessions: 0, + userDailyResetMode: "rolling", + userDailyResetTime: "00:00", + userIsEnabled: true, + userExpiresAt: null, + userAllowedClients: [], + userAllowedModels: [], + userCreatedAt: new Date("2024-01-01T00:00:00.000Z"), + userUpdatedAt: new Date("2024-01-01T00:00:00.000Z"), + userDeletedAt: null, + }; + return { ...base, ...overrides }; +} + +describe("repository/key resolveApiKeyAuthOutcome", () => { + beforeEach(() => { + selectMock.mockClear(); + fromMock.mockClear(); + innerJoinMock.mockClear(); + whereMock.mockReset(); + }); + + it("returns ok=true with hydrated user/key when the row is fully active", async () => { + whereMock.mockResolvedValueOnce([activeRow()]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-test"); + + expect(outcome.ok).toBe(true); + if (outcome.ok) { + expect(outcome.user.id).toBe(2); + expect(outcome.key.id).toBe(1); + } + }); + + it("returns not_found when the row does not exist", async () => { + whereMock.mockResolvedValueOnce([]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-missing"); + + expect(outcome).toEqual({ ok: false, reason: "not_found" }); + }); + + it("returns key_disabled when the row exists but isEnabled=false", async () => { + whereMock.mockResolvedValueOnce([activeRow({ keyIsEnabled: false })]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-disabled"); + + expect(outcome).toEqual({ ok: false, reason: "key_disabled" }); + }); + + it("returns key_expired when expiresAt is in the past", async () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000); + whereMock.mockResolvedValueOnce([activeRow({ keyExpiresAt: yesterday })]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-expired"); + + expect(outcome).toEqual({ ok: false, reason: "key_expired" }); + }); + + it("prefers key_disabled over key_expired when both are true", async () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000); + whereMock.mockResolvedValueOnce([activeRow({ keyIsEnabled: false, keyExpiresAt: yesterday })]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-both"); + + expect(outcome).toEqual({ ok: false, reason: "key_disabled" }); + }); + + it("back-compat wrapper validateApiKeyAndGetUser returns null on any failure", async () => { + whereMock.mockResolvedValueOnce([activeRow({ keyIsEnabled: false })]); + + const { validateApiKeyAndGetUser } = await import("@/repository/key"); + const result = await validateApiKeyAndGetUser("sk-disabled"); + + expect(result).toBeNull(); + }); +}); From b5d7449d87c2e1006049dfcea5d002f60a0242a4 Mon Sep 17 00:00:00 2001 From: ding113 Date: Thu, 14 May 2026 23:35:50 +0800 Subject: [PATCH 2/6] perf(availability): inline finalized predicate to restore index use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The availability dashboard query was slow because the WHERE clause called the PL/pgSQL function `fn_is_message_request_finalized(...)`. PostgreSQL never inlines PL/pgSQL functions, so the predicate became opaque to the planner and the partial index `idx_message_request_provider_created_at_finalized_active` (predicate `status_code IS NOT NULL AND deleted_at IS NULL`) was no longer usable — the dashboard fell back to a sequential scan that re-evaluated the function per row. Inline a semantically-equivalent SQL expression so `status_code IS NOT NULL` becomes the dominant SARGable branch. The SQL function definition is unchanged (still called by the upsert trigger on the write path); a header comment marks the keep-in-sync requirement. Tests assert on the inlined form to guard against the slow function-call form returning. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/availability/availability-service.ts | 50 ++++++++++++++++---- tests/unit/lib/availability-service.test.ts | 30 ++++++++++-- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/src/lib/availability/availability-service.ts b/src/lib/availability/availability-service.ts index 4d5e6f0bd..ee0ca46cb 100644 --- a/src/lib/availability/availability-service.ts +++ b/src/lib/availability/availability-service.ts @@ -62,18 +62,50 @@ export class AvailabilityQueryValidationError extends Error { } /** - * 当前版本把“已终态”收敛为 `statusCode` 已落库。 + * "Finalized request" predicate used in the availability CTE WHERE clause. * - * 已知限制：在当前异步写入/丢 patch 的极端场景，或未来新增了 `durationMs` / `errorMessage` - * 已落库、但 `statusCode` 仍为空且已稳定结束的写路径时，这些记录会被当前可用性统计排除。 - * 届时应引入独立的 finalized 谓词，而不是直接放宽为 `durationMs IS NOT NULL`。 + * SEMANTICALLY EQUIVALENT to `fn_is_message_request_finalized(blocked_by, + * status_code, provider_chain, error_message)` defined in drizzle/0095_*.sql + * (re-affirmed in 0097_*.sql / 0098_*.sql). It is intentionally inlined here + * because PostgreSQL does NOT inline PL/pgSQL functions, which means calling + * the function in the WHERE clause makes the predicate opaque to the + * planner. That hides the dominant `status_code IS NOT NULL` branch and + * prevents the planner from using the partial index + * `idx_message_request_provider_created_at_finalized_active` + * (predicate: `status_code IS NOT NULL AND deleted_at IS NULL`), which + * collapses the dashboard query into a sequential scan. + * + * KEEP IN SYNC with `fn_is_message_request_finalized` in + * drizzle/0095_young_lily_hollister.sql; the trigger and the row-level + * outcome function still call the SQL function (per-row write path, not + * latency critical), so the canonical definition stays in PL/pgSQL. */ function buildAvailabilityFinalizedCondition() { - return sql`fn_is_message_request_finalized( - ${messageRequest.blockedBy}, - ${messageRequest.statusCode}, - ${messageRequest.providerChain}, - ${messageRequest.errorMessage} + // The `IS NOT NULL` checks below are individually SARGable. Listing + // status_code first encourages the planner to scan the partial index. + return sql`( + ${messageRequest.statusCode} IS NOT NULL + OR ${messageRequest.blockedBy} IS NOT NULL + OR COALESCE(${messageRequest.errorMessage}, '') <> '' + OR ( + ${messageRequest.providerChain} IS NOT NULL + AND jsonb_typeof(${messageRequest.providerChain}) = 'array' + AND jsonb_array_length(${messageRequest.providerChain}) > 0 + AND jsonb_typeof(${messageRequest.providerChain} -> -1) = 'object' + AND ( + (${messageRequest.providerChain} -> -1 ->> 'reason') IN ( + 'request_success', 'retry_success', 'retry_failed', 'system_error', + 'resource_not_found', 'client_error_non_retryable', + 'concurrent_limit_failed', 'hedge_winner', 'hedge_loser_cancelled', + 'client_abort' + ) + OR ( + (${messageRequest.providerChain} -> -1 ? 'statusCode') + AND jsonb_typeof(${messageRequest.providerChain} -> -1 -> 'statusCode') = 'number' + ) + OR COALESCE(${messageRequest.providerChain} -> -1 ->> 'errorMessage', '') <> '' + ) + ) )`; } diff --git a/tests/unit/lib/availability-service.test.ts b/tests/unit/lib/availability-service.test.ts index ac14b0013..157962d04 100644 --- a/tests/unit/lib/availability-service.test.ts +++ b/tests/unit/lib/availability-service.test.ts @@ -309,7 +309,13 @@ describe("availability-service", () => { const queryText = normalizeSql(executeMock.mock.calls[0]?.[0]); const finalizedRequestsSql = extractFinalizedRequestsSql(queryText); - expect(finalizedRequestsSql).toContain("fn_is_message_request_finalized"); + // The "finalized" predicate is inlined as a SARGable expression (not a + // function call) so the planner can use the partial index on + // status_code IS NOT NULL. + expect(finalizedRequestsSql).not.toContain("fn_is_message_request_finalized"); + expect(finalizedRequestsSql).toContain(`"status_code" is not null`); + expect(finalizedRequestsSql).toContain(`"blocked_by" is not null`); + expect(finalizedRequestsSql).toContain(`"provider_chain" -> -1 ->> 'reason'`); expect(queryText).toContain("group by"); expect(queryText).toContain("percentile_cont(0.95)"); expect(queryText).toContain("row_number() over"); @@ -482,7 +488,13 @@ describe("availability-service", () => { const finalizedRequestsSql = extractFinalizedRequestsSql( normalizeSql(executeMock.mock.calls[0]?.[0]) ); - expect(finalizedRequestsSql).toContain("fn_is_message_request_finalized"); + // The "finalized" predicate is inlined as a SARGable expression (not a + // function call) so the planner can use the partial index on + // status_code IS NOT NULL. + expect(finalizedRequestsSql).not.toContain("fn_is_message_request_finalized"); + expect(finalizedRequestsSql).toContain(`"status_code" is not null`); + expect(finalizedRequestsSql).toContain(`"blocked_by" is not null`); + expect(finalizedRequestsSql).toContain(`"provider_chain" -> -1 ->> 'reason'`); }); it("queryProviderAvailability 会保留 Gemini passthrough 终态(statusCode!=null 且 durationMs=null)", async () => { @@ -548,7 +560,13 @@ describe("availability-service", () => { const queryText = normalizeSql(executeMock.mock.calls[0]?.[0]); const finalizedRequestsSql = extractFinalizedRequestsSql(queryText); - expect(finalizedRequestsSql).toContain("fn_is_message_request_finalized"); + // The "finalized" predicate is inlined as a SARGable expression (not a + // function call) so the planner can use the partial index on + // status_code IS NOT NULL. + expect(finalizedRequestsSql).not.toContain("fn_is_message_request_finalized"); + expect(finalizedRequestsSql).toContain(`"status_code" is not null`); + expect(finalizedRequestsSql).toContain(`"blocked_by" is not null`); + expect(finalizedRequestsSql).toContain(`"provider_chain" -> -1 ->> 'reason'`); expect(queryText).toContain("fn_compute_message_request_success_rate_outcome"); expect(queryText).toContain(`"successrateoutcome" = 'failure'`); }); @@ -717,7 +735,11 @@ describe("availability-service", () => { ]); const queryText = normalizeSql(executeMock.mock.calls[0]?.[0]); - expect(queryText).toContain("fn_is_message_request_finalized"); + // Inlined finalized predicate (planner-transparent; see + // buildAvailabilityFinalizedCondition in availability-service.ts). + expect(queryText).not.toContain("fn_is_message_request_finalized"); + expect(queryText).toContain(`"status_code" is not null`); + expect(queryText).toContain(`"blocked_by" is not null`); expect(queryText).toContain(">= now() - (15 * interval '1 minute')"); expect(queryText).toContain("<= now()"); expect(queryText).toContain("count(*) filter"); From d314561339a28903c9f942f3fc0fbb76d0265c9b Mon Sep 17 00:00:00 2001 From: ding113 Date: Fri, 15 May 2026 00:07:03 +0800 Subject: [PATCH 3/6] fix(auth,availability): address PR #1187 review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - key.ts: classify duplicate-row matches deterministically. The relaxed WHERE clause can return multiple non-deleted rows for one key string (no unique constraint on keys.key) and result[0] was non-deterministic; prefer an active row, fall back to "any enabled = key_expired", else key_disabled. (chatgpt-codex P1) - availability-service.ts: wrap the provider_chain jsonb branch in a CASE expression so jsonb_array_length cannot run on a non-array row. PostgreSQL does not guarantee AND short-circuit, so a single non-array historical row would otherwise crash the dashboard query. Extract the finalized provider_chain reason list into FINALIZED_PROVIDER_CHAIN_REASONS and document the JSONB `?` operator's driver assumption. (coderabbit P2, gemini, greptile) - auth-guard.ts / available-models.ts: convert the outcome.reason branch to an exhaustive switch with assertNever, and introduce a buildAuthFailure factory so every failure path is forced to tag its failureKind at compile time. Adding a new ApiKeyAuthFailureReason now produces a TypeScript error until the new branch is handled. (greptile P2 ×2) - Tests cover the duplicate-row cases (ok / key_expired / key_disabled across mixed-state rows) and assert the CASE guard appears in the generated SQL. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/v1/_lib/models/available-models.ts | 21 ++- src/app/v1/_lib/proxy/auth-guard.ts | 167 ++++++++++-------- src/lib/availability/availability-service.ts | 66 +++++-- src/repository/key.ts | 23 ++- tests/unit/lib/availability-service.test.ts | 7 + .../repository/key-resolve-outcome.test.ts | 64 ++++++- 6 files changed, 242 insertions(+), 106 deletions(-) diff --git a/src/app/v1/_lib/models/available-models.ts b/src/app/v1/_lib/models/available-models.ts index 0538ec225..c09eebcb6 100644 --- a/src/app/v1/_lib/models/available-models.ts +++ b/src/app/v1/_lib/models/available-models.ts @@ -62,14 +62,21 @@ async function authenticateRequest(c: Context): Promise<{ const outcome = await resolveApiKeyAuthOutcome(apiKey); if (!outcome.ok) { - if (outcome.reason === "key_disabled") { - throw c.json({ error: { message: "API 密钥已被禁用", type: "key_disabled" } }, 401); - } - if (outcome.reason === "key_expired") { - throw c.json({ error: { message: "API 密钥已过期", type: "key_expired" } }, 401); + // Exhaustive switch: see auth-guard.ts for rationale. Adding a new + // ApiKeyAuthFailureReason will produce a TypeScript error on the + // exhaustiveness fallthrough until this branch is handled explicitly. + switch (outcome.reason) { + case "key_disabled": + throw c.json({ error: { message: "API 密钥已被禁用", type: "key_disabled" } }, 401); + case "key_expired": + throw c.json({ error: { message: "API 密钥已过期", type: "key_expired" } }, 401); + case "not_found": + throw c.json({ error: { message: "API 密钥无效", type: "invalid_api_key" } }, 401); + default: { + const _exhaustive: never = outcome.reason; + throw new Error(`Unhandled auth outcome reason: ${JSON.stringify(_exhaustive)}`); + } } - // not_found - throw c.json({ error: { message: "API 密钥无效", type: "invalid_api_key" } }, 401); } const { user, key } = outcome; diff --git a/src/app/v1/_lib/proxy/auth-guard.ts b/src/app/v1/_lib/proxy/auth-guard.ts index 27eb7433b..02d1f858a 100644 --- a/src/app/v1/_lib/proxy/auth-guard.ts +++ b/src/app/v1/_lib/proxy/auth-guard.ts @@ -6,7 +6,38 @@ import { resolveApiKeyAuthOutcome } from "@/repository/key"; import { markUserExpired } from "@/repository/user"; import { GEMINI_PROTOCOL } from "../gemini/protocol"; import { ProxyResponses } from "./responses"; -import type { AuthState, ProxySession } from "./session"; +import type { AuthFailureKind, AuthState, ProxySession } from "./session"; + +/** + * Build an auth failure AuthState. The factory exists so every call site is + * forced (by the function signature) to supply both `failureKind` and an + * `errorResponse` — preventing the footgun where a new failure branch could + * forget to tag itself and silently get classified as a credentials failure + * by the brute-force rate limiter. + */ +function buildAuthFailure(params: { + failureKind: AuthFailureKind; + errorResponse: Response; + apiKey?: string | null; +}): AuthState { + return { + user: null, + key: null, + apiKey: params.apiKey ?? null, + success: false, + failureKind: params.failureKind, + errorResponse: params.errorResponse, + }; +} + +/** + * Exhaustiveness helper. Calling this from a "should be unreachable" branch + * gives a compile-time error when a new union member is added without an + * explicit handler. + */ +function assertNever(value: never, context: string): never { + throw new Error(`Unhandled discriminant in ${context}: ${JSON.stringify(value)}`); +} /** * Pre-auth rate limiter: throttles repeated authentication failures per IP @@ -133,18 +164,14 @@ export class ProxyAuthenticator { hasGeminiApiKeyHeader: !!headers.geminiApiKeyHeader, hasGeminiApiKeyQuery: !!headers.geminiApiKeyQuery, }); - return { - user: null, - key: null, - apiKey: null, - success: false, + return buildAuthFailure({ failureKind: "credentials", errorResponse: ProxyResponses.buildError( 401, "未提供认证凭据。请在 Authorization 头部、x-api-key 头部或 x-goog-api-key 头部中包含 API 密钥。", "authentication_error" ), - }; + }); } const [firstKey] = providedKeys; @@ -154,79 +181,73 @@ export class ProxyAuthenticator { logger.warn("[ProxyAuthenticator] Multiple conflicting API keys provided", { keyCount: providedKeys.length, }); - return { - user: null, - key: null, - apiKey: null, - success: false, + return buildAuthFailure({ failureKind: "credentials", errorResponse: ProxyResponses.buildError( 401, "提供了多个冲突的 API 密钥。请仅使用一种认证方式。", "authentication_error" ), - }; + }); } const apiKey = firstKey; const outcome = await resolveApiKeyAuthOutcome(apiKey); if (!outcome.ok) { - if (outcome.reason === "not_found") { - logger.debug("[ProxyAuthenticator] API key validation failed: not found", { - apiKeyLength: apiKey.length, - fromHeader: - !!headers.authHeader || !!headers.apiKeyHeader || !!headers.geminiApiKeyHeader, - fromQuery: !!headers.geminiApiKeyQuery, - }); - return { - user: null, - key: null, - apiKey, - success: false, - failureKind: "credentials", - errorResponse: ProxyResponses.buildError( - 401, - "API 密钥无效。提供的密钥不存在或已被删除。", - "invalid_api_key" - ), - }; - } + // Exhaustive switch: adding a new ApiKeyAuthFailureReason in the + // repository layer will trigger a compile error in assertNever below + // until this guard is updated, preventing a new variant from silently + // falling into the wrong branch. + switch (outcome.reason) { + case "not_found": + logger.debug("[ProxyAuthenticator] API key validation failed: not found", { + apiKeyLength: apiKey.length, + fromHeader: + !!headers.authHeader || !!headers.apiKeyHeader || !!headers.geminiApiKeyHeader, + fromQuery: !!headers.geminiApiKeyQuery, + }); + return buildAuthFailure({ + apiKey, + failureKind: "credentials", + errorResponse: ProxyResponses.buildError( + 401, + "API 密钥无效。提供的密钥不存在或已被删除。", + "invalid_api_key" + ), + }); - if (outcome.reason === "key_disabled") { - logger.warn("[ProxyAuthenticator] API key is disabled", { - apiKeyLength: apiKey.length, - }); - return { - user: null, - key: null, - apiKey, - success: false, - failureKind: "account_state", - errorResponse: ProxyResponses.buildError( - 401, - "API 密钥已被禁用。请联系管理员重新启用，或使用其他可用密钥。", - "key_disabled" - ), - }; - } + case "key_disabled": + logger.warn("[ProxyAuthenticator] API key is disabled", { + apiKeyLength: apiKey.length, + }); + return buildAuthFailure({ + apiKey, + failureKind: "account_state", + errorResponse: ProxyResponses.buildError( + 401, + "API 密钥已被禁用。请联系管理员重新启用，或使用其他可用密钥。", + "key_disabled" + ), + }); - // outcome.reason === "key_expired" - logger.warn("[ProxyAuthenticator] API key has expired", { - apiKeyLength: apiKey.length, - }); - return { - user: null, - key: null, - apiKey, - success: false, - failureKind: "account_state", - errorResponse: ProxyResponses.buildError( - 401, - "API 密钥已过期。请联系管理员续期或更换密钥。", - "key_expired" - ), - }; + case "key_expired": + logger.warn("[ProxyAuthenticator] API key has expired", { + apiKeyLength: apiKey.length, + }); + return buildAuthFailure({ + apiKey, + failureKind: "account_state", + errorResponse: ProxyResponses.buildError( + 401, + "API 密钥已过期。请联系管理员续期或更换密钥。", + "key_expired" + ), + }); + + default: + assertNever(outcome.reason, "ProxyAuthenticator.validate outcome.reason"); + } } // Check user status and expiration @@ -238,18 +259,15 @@ export class ProxyAuthenticator { userId: user.id, userName: user.name, }); - return { - user: null, - key: null, + return buildAuthFailure({ apiKey, - success: false, failureKind: "account_state", errorResponse: ProxyResponses.buildError( 401, "用户账户已被禁用。请联系管理员。", "user_disabled" ), - }; + }); } // 2. Check if user is expired (lazy expiration check) @@ -266,18 +284,15 @@ export class ProxyAuthenticator { error: error instanceof Error ? error.message : String(error), }); }); - return { - user: null, - key: null, + return buildAuthFailure({ apiKey, - success: false, failureKind: "account_state", errorResponse: ProxyResponses.buildError( 401, `用户账户已于 ${user.expiresAt.toISOString().split("T")[0]} 过期。请续费订阅。`, "user_expired" ), - }; + }); } logger.debug("[ProxyAuthenticator] Authentication successful", { diff --git a/src/lib/availability/availability-service.ts b/src/lib/availability/availability-service.ts index ee0ca46cb..8e0560b0f 100644 --- a/src/lib/availability/availability-service.ts +++ b/src/lib/availability/availability-service.ts @@ -45,6 +45,29 @@ const AVAILABILITY_SUCCESS_STATUS_CODE_MAX_EXCLUSIVE = 400; const FINALIZED_REQUEST_OUTCOME_ALIAS = "successRateOutcome" as const; const FINALIZED_REQUEST_OUTCOME_SQL = sql.raw(`"${FINALIZED_REQUEST_OUTCOME_ALIAS}"`); const COUNTABLE_REQUEST_OUTCOME_SQL = sql`${FINALIZED_REQUEST_OUTCOME_SQL} IN ('success', 'failure')`; + +/** + * Provider-chain `reason` values that, when present on the last chain entry, + * indicate the message-request has reached a terminal state. + * + * Mirrors the list inside `fn_is_message_request_finalized` (drizzle/0095_*.sql) + * and `fn_compute_message_request_success_rate_outcome` — keep in sync. + */ +const FINALIZED_PROVIDER_CHAIN_REASONS = [ + "request_success", + "retry_success", + "retry_failed", + "system_error", + "resource_not_found", + "client_error_non_retryable", + "concurrent_limit_failed", + "hedge_winner", + "hedge_loser_cancelled", + "client_abort", +] as const; +const FINALIZED_PROVIDER_CHAIN_REASONS_SQL = sql.raw( + FINALIZED_PROVIDER_CHAIN_REASONS.map((reason) => `'${reason}'`).join(", ") +); // Keep the hard cap independent from the UI/API default so future default tuning does not silently relax/tighten the guardrail. // It intentionally equals the default today; the separation preserves distinct semantic roles for future tuning. export const MAX_BUCKETS_HARD_LIMIT = 100; @@ -83,28 +106,39 @@ export class AvailabilityQueryValidationError extends Error { function buildAvailabilityFinalizedCondition() { // The `IS NOT NULL` checks below are individually SARGable. Listing // status_code first encourages the planner to scan the partial index. + // + // The provider_chain branch wraps each jsonb-array operation in a CASE + // because PostgreSQL does NOT guarantee left-to-right short-circuit of + // AND / OR (see PG docs on Logical Operators). Without CASE, an + // observed-rare-but-legal historical row where `provider_chain` is a + // non-array jsonb value (object, scalar, or json null) would make + // `jsonb_array_length(...)` raise `cannot get array length of a non-array` + // and crash the dashboard query. + // + // The `?` JSONB key-existence operator on the last line is correct under + // the `pg` driver Drizzle uses today (parameterized via `$N`). If we ever + // swap drivers (e.g. `postgres.js`) bare `?` may be reinterpreted as a + // positional placeholder; either change the driver or use + // `jsonb_exists(..., 'statusCode')` at that point. return sql`( ${messageRequest.statusCode} IS NOT NULL OR ${messageRequest.blockedBy} IS NOT NULL OR COALESCE(${messageRequest.errorMessage}, '') <> '' OR ( - ${messageRequest.providerChain} IS NOT NULL - AND jsonb_typeof(${messageRequest.providerChain}) = 'array' - AND jsonb_array_length(${messageRequest.providerChain}) > 0 - AND jsonb_typeof(${messageRequest.providerChain} -> -1) = 'object' - AND ( - (${messageRequest.providerChain} -> -1 ->> 'reason') IN ( - 'request_success', 'retry_success', 'retry_failed', 'system_error', - 'resource_not_found', 'client_error_non_retryable', - 'concurrent_limit_failed', 'hedge_winner', 'hedge_loser_cancelled', - 'client_abort' - ) - OR ( - (${messageRequest.providerChain} -> -1 ? 'statusCode') - AND jsonb_typeof(${messageRequest.providerChain} -> -1 -> 'statusCode') = 'number' + CASE + WHEN ${messageRequest.providerChain} IS NULL THEN FALSE + WHEN jsonb_typeof(${messageRequest.providerChain}) <> 'array' THEN FALSE + WHEN jsonb_array_length(${messageRequest.providerChain}) = 0 THEN FALSE + WHEN jsonb_typeof(${messageRequest.providerChain} -> -1) <> 'object' THEN FALSE + ELSE ( + (${messageRequest.providerChain} -> -1 ->> 'reason') IN (${FINALIZED_PROVIDER_CHAIN_REASONS_SQL}) + OR ( + (${messageRequest.providerChain} -> -1 ? 'statusCode') + AND jsonb_typeof(${messageRequest.providerChain} -> -1 -> 'statusCode') = 'number' + ) + OR COALESCE(${messageRequest.providerChain} -> -1 ->> 'errorMessage', '') <> '' ) - OR COALESCE(${messageRequest.providerChain} -> -1 ->> 'errorMessage', '') <> '' - ) + END ) )`; } diff --git a/src/repository/key.ts b/src/repository/key.ts index e0ee26384..029573145 100644 --- a/src/repository/key.ts +++ b/src/repository/key.ts @@ -693,15 +693,26 @@ export async function resolveApiKeyAuthOutcome(keyString: string): Promise expired > disabled. + const now = Date.now(); + const activeRow = result.find( + (candidate) => + candidate.keyIsEnabled === true && + (!candidate.keyExpiresAt || candidate.keyExpiresAt.getTime() > now) + ); - // 区分失败原因：先 disabled，再 expired，再 ok。 - if (row.keyIsEnabled !== true) { + if (!activeRow) { + const expiredRow = result.find((candidate) => candidate.keyIsEnabled === true); + if (expiredRow) { + return { ok: false, reason: "key_expired" }; + } return { ok: false, reason: "key_disabled" }; } - if (row.keyExpiresAt && row.keyExpiresAt.getTime() <= Date.now()) { - return { ok: false, reason: "key_expired" }; - } + + const row = activeRow; const user: User = toUser({ id: row.userId, diff --git a/tests/unit/lib/availability-service.test.ts b/tests/unit/lib/availability-service.test.ts index 157962d04..8a8d38a8e 100644 --- a/tests/unit/lib/availability-service.test.ts +++ b/tests/unit/lib/availability-service.test.ts @@ -316,6 +316,13 @@ describe("availability-service", () => { expect(finalizedRequestsSql).toContain(`"status_code" is not null`); expect(finalizedRequestsSql).toContain(`"blocked_by" is not null`); expect(finalizedRequestsSql).toContain(`"provider_chain" -> -1 ->> 'reason'`); + // The provider_chain branch must wrap jsonb operations in a CASE so the + // dashboard query does not crash on a non-array historical row + // (PostgreSQL does not guarantee AND short-circuit). + expect(finalizedRequestsSql).toContain("case"); + expect(finalizedRequestsSql).toContain( + `jsonb_typeof("message_request"."provider_chain") <> 'array'` + ); expect(queryText).toContain("group by"); expect(queryText).toContain("percentile_cont(0.95)"); expect(queryText).toContain("row_number() over"); diff --git a/tests/unit/repository/key-resolve-outcome.test.ts b/tests/unit/repository/key-resolve-outcome.test.ts index b8d2d23ab..8aa2b3c1a 100644 --- a/tests/unit/repository/key-resolve-outcome.test.ts +++ b/tests/unit/repository/key-resolve-outcome.test.ts @@ -170,11 +170,23 @@ function activeRow(overrides: Partial> = {}) { } describe("repository/key resolveApiKeyAuthOutcome", () => { - beforeEach(() => { + beforeEach(async () => { selectMock.mockClear(); fromMock.mockClear(); innerJoinMock.mockClear(); whereMock.mockReset(); + + // vitest config sets `mockReset: true` globally, which strips the + // mockResolvedValue implementation off the hoisted cache mocks between + // tests. Re-apply it here so success-path tests don't trip on + // `cacheAuthResult(...).catch(...)` when the returned promise is gone. + const cache = await import("@/lib/security/api-key-auth-cache"); + vi.mocked(cache.getCachedActiveKey).mockResolvedValue(null); + vi.mocked(cache.getCachedUser).mockResolvedValue(null); + vi.mocked(cache.cacheActiveKey).mockResolvedValue(undefined); + vi.mocked(cache.cacheAuthResult).mockResolvedValue(undefined); + vi.mocked(cache.cacheUser).mockResolvedValue(undefined); + vi.mocked(cache.invalidateCachedKey).mockResolvedValue(undefined); }); it("returns ok=true with hydrated user/key when the row is fully active", async () => { @@ -236,4 +248,54 @@ describe("repository/key resolveApiKeyAuthOutcome", () => { expect(result).toBeNull(); }); + + // `keys.key` has no unique constraint, so multiple non-deleted rows may + // share a key string. The classifier MUST prefer an active row to avoid + // mis-rejecting a valid credential as disabled/expired. + describe("deterministic classification across duplicate rows", () => { + it("returns ok=true when at least one duplicate row is active", async () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000); + whereMock.mockResolvedValueOnce([ + // A disabled duplicate sorted before an active one — pre-fix this + // would have returned key_disabled and locked the owner out. + activeRow({ keyId: 10, keyIsEnabled: false }), + activeRow({ keyId: 11, keyIsEnabled: true, keyExpiresAt: null }), + activeRow({ keyId: 12, keyIsEnabled: true, keyExpiresAt: yesterday }), + ]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-dup-mixed"); + + expect(outcome.ok).toBe(true); + if (outcome.ok) { + expect(outcome.key.id).toBe(11); + } + }); + + it("returns key_expired when no row is active but at least one is enabled", async () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000); + whereMock.mockResolvedValueOnce([ + activeRow({ keyId: 20, keyIsEnabled: false }), + activeRow({ keyId: 21, keyIsEnabled: true, keyExpiresAt: yesterday }), + ]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-dup-expired"); + + expect(outcome).toEqual({ ok: false, reason: "key_expired" }); + }); + + it("returns key_disabled when every duplicate row is disabled", async () => { + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000); + whereMock.mockResolvedValueOnce([ + activeRow({ keyId: 30, keyIsEnabled: false }), + activeRow({ keyId: 31, keyIsEnabled: false, keyExpiresAt: yesterday }), + ]); + + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const outcome = await resolveApiKeyAuthOutcome("sk-dup-disabled"); + + expect(outcome).toEqual({ ok: false, reason: "key_disabled" }); + }); + }); }); From 26d3b9f0e895a274739fc479ef0c6d3d5524f9bf Mon Sep 17 00:00:00 2001 From: ding113 Date: Fri, 15 May 2026 00:20:03 +0800 Subject: [PATCH 4/6] fix(public-status): expire config snapshots so Redis stops accumulating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit publishPublicStatusConfigSnapshot, publishInternalPublicStatusConfigSnapshot, and publishCurrentPublicStatusConfigPointers all wrote keys with bare redis.set(...) and no TTL. Every config-version mint (provider/group/system settings change) created a new versioned snapshot key that never expired — on a busy operator the public-status:v1:config:* and config-internal:* key namespaces grow without bound. Neighbouring projection writers in rebuild-worker.ts already use a 30-day TTL via setWithTtl; only the config publishing path was missed when that pattern was introduced (#1056). Add PUBLIC_STATUS_CONFIG_TTL_SECONDS (30 days, matching GENERATION_PROJECTION_TTL_SECONDS in rebuild-worker.ts), widen the local RedisWriter type to the (key, value, "EX", seconds) ioredis overload, and apply the TTL to all four call sites — including the Lua script used by the pointer publisher so SET ... EX is atomic with the version compare. Each successful publish refreshes the TTL on the live pointer keys, so as long as configs are published at least every 30 days the active pointer never expires while stale versioned snapshots get cleaned up naturally. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/public-status/config-snapshot.ts | 45 ++++- .../public-status/config-snapshot.test.ts | 154 +++++++++++++++++- 2 files changed, 190 insertions(+), 9 deletions(-) diff --git a/src/lib/public-status/config-snapshot.ts b/src/lib/public-status/config-snapshot.ts index 6a76582d2..4fe313d8a 100644 --- a/src/lib/public-status/config-snapshot.ts +++ b/src/lib/public-status/config-snapshot.ts @@ -8,6 +8,21 @@ import { export const DEFAULT_PUBLIC_STATUS_SITE_DESCRIPTION = "Request-derived public status"; +/** + * TTL (seconds) applied to every Redis key written by this module. + * + * 30 days matches `GENERATION_PROJECTION_TTL_SECONDS` in `rebuild-worker.ts`, + * which already governs the manifest / series / snapshot keys for this + * feature. Versioned config snapshot keys + * (`public-status:v1:config:` and the internal variant) are written + * once per config publication; without a TTL they accumulate indefinitely as + * provider/group/system-settings changes mint new config versions. Each + * publish refreshes the TTL on the "current" pointer keys, so as long as + * configurations are published at least every 30 days the live pointers + * never expire while stale versioned snapshots get cleaned up naturally. + */ +export const PUBLIC_STATUS_CONFIG_TTL_SECONDS = 60 * 60 * 24 * 30; + export interface PublicStatusModelSnapshot { publicModelKey: string; label: string; @@ -67,6 +82,10 @@ interface BuildPublicStatusConfigSnapshotInput { } interface RedisWriter { + // ioredis supports both bare `set(key, value)` and the EX-variant + // `set(key, value, "EX", seconds)`. Widening the type here lets us pass + // an explicit TTL on every write — see PUBLIC_STATUS_CONFIG_TTL_SECONDS. + set(key: string, value: string, mode: "EX", seconds: number): Promise | unknown; set(key: string, value: string): Promise | unknown; get?(key: string): Promise | string | null; eval?(script: string, numKeys: number, ...args: string[]): Promise | unknown; @@ -205,11 +224,13 @@ export async function publishPublicStatusConfigSnapshot(input: { const redis = input.redis ?? getRedisClient({ allowWhenRateLimitDisabled: true }); if (redis) { - await redis.set(key, JSON.stringify(snapshot)); + await redis.set(key, JSON.stringify(snapshot), "EX", PUBLIC_STATUS_CONFIG_TTL_SECONDS); if (input.setCurrentPointer !== false) { await redis.set( buildPublicStatusConfigSnapshotKey(), - JSON.stringify({ key, configVersion: snapshot.configVersion }) + JSON.stringify({ key, configVersion: snapshot.configVersion }), + "EX", + PUBLIC_STATUS_CONFIG_TTL_SECONDS ); } } @@ -242,11 +263,13 @@ export async function publishInternalPublicStatusConfigSnapshot(input: { const redis = input.redis ?? getRedisClient({ allowWhenRateLimitDisabled: true }); if (redis) { - await redis.set(key, JSON.stringify(input.snapshot)); + await redis.set(key, JSON.stringify(input.snapshot), "EX", PUBLIC_STATUS_CONFIG_TTL_SECONDS); if (input.setCurrentPointer !== false) { await redis.set( buildPublicStatusInternalConfigSnapshotKey(), - JSON.stringify({ key, configVersion: input.snapshot.configVersion }) + JSON.stringify({ key, configVersion: input.snapshot.configVersion }), + "EX", + PUBLIC_STATUS_CONFIG_TTL_SECONDS ); } } @@ -269,15 +292,23 @@ export async function publishCurrentPublicStatusConfigPointers(input: { const pointerKey = buildPublicStatusConfigVersionPointerKey(); if (typeof redis.eval === "function") { + // SET ... EX applies the TTL atomically with the write, refreshing + // the expiration on every successful publish. const luaScript = ` local current = redis.call('GET', KEYS[1]) if (not current) or current <= ARGV[1] then - redis.call('SET', KEYS[1], ARGV[1]) + redis.call('SET', KEYS[1], ARGV[1], 'EX', ARGV[2]) return 1 end return 0 `; - const result = await redis.eval(luaScript, 1, pointerKey, input.configVersion); + const result = await redis.eval( + luaScript, + 1, + pointerKey, + input.configVersion, + String(PUBLIC_STATUS_CONFIG_TTL_SECONDS) + ); return result === 1; } @@ -289,7 +320,7 @@ export async function publishCurrentPublicStatusConfigPointers(input: { return false; } - await redis.set(pointerKey, input.configVersion); + await redis.set(pointerKey, input.configVersion, "EX", PUBLIC_STATUS_CONFIG_TTL_SECONDS); return true; } diff --git a/tests/unit/public-status/config-snapshot.test.ts b/tests/unit/public-status/config-snapshot.test.ts index f8e57c7cf..3b896f1aa 100644 --- a/tests/unit/public-status/config-snapshot.test.ts +++ b/tests/unit/public-status/config-snapshot.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { importPublicStatusModule } from "../../helpers/public-status-test-helpers"; interface ConfigSnapshotModule { + PUBLIC_STATUS_CONFIG_TTL_SECONDS: number; buildPublicStatusConfigSnapshot(input: { configVersion: string; siteTitle: string; @@ -43,11 +44,44 @@ interface ConfigSnapshotModule { get: (key: string) => Promise; }; }): Promise<{ siteTitle: string; siteDescription: string } | null>; + publishPublicStatusConfigSnapshot(input: { + reason: string; + snapshot?: { + configVersion: string; + generatedAt: string; + siteTitle: string; + siteDescription: string; + timeZone: string | null; + defaultIntervalMinutes: number; + defaultRangeHours: number; + groups: unknown[]; + }; + redis: { + set: (...args: unknown[]) => Promise; + }; + setCurrentPointer?: boolean; + }): Promise<{ configVersion: string; key: string; written: boolean }>; + publishInternalPublicStatusConfigSnapshot(input: { + snapshot: { + configVersion: string; + generatedAt: string; + siteTitle: string; + siteDescription: string; + timeZone: string | null; + defaultIntervalMinutes: number; + defaultRangeHours: number; + groups: unknown[]; + }; + redis: { + set: (...args: unknown[]) => Promise; + }; + setCurrentPointer?: boolean; + }): Promise<{ configVersion: string; key: string; written: boolean }>; publishCurrentPublicStatusConfigPointers(input: { configVersion: string; redis: { - set: (key: string, value: string) => Promise; - eval: (script: string, numKeys: number, ...args: string[]) => Promise; + set: (...args: unknown[]) => Promise; + eval?: (script: string, numKeys: number, ...args: string[]) => Promise; }; }): Promise; } @@ -151,4 +185,120 @@ describe("public-status config snapshot", () => { ).resolves.toBe(false); expect(redis.set).not.toHaveBeenCalled(); }); + + // Regression: before this guard, every config publish wrote Redis keys + // without TTL, so versioned snapshot keys accumulated forever as + // provider/group/system-settings changes minted new config versions. + describe("Redis writes apply PUBLIC_STATUS_CONFIG_TTL_SECONDS", () => { + it("publishPublicStatusConfigSnapshot writes both the versioned key and the current pointer with TTL", async () => { + const mod = await importPublicStatusModule( + "@/lib/public-status/config-snapshot" + ); + const ttl = mod.PUBLIC_STATUS_CONFIG_TTL_SECONDS; + expect(ttl).toBeGreaterThan(0); + + const redis = { set: vi.fn().mockResolvedValue("OK") }; + + await mod.publishPublicStatusConfigSnapshot({ + reason: "test", + snapshot: { + configVersion: "cfg-2", + generatedAt: new Date().toISOString(), + siteTitle: "Test", + siteDescription: "Test", + timeZone: null, + defaultIntervalMinutes: 5, + defaultRangeHours: 24, + groups: [], + }, + redis, + }); + + expect(redis.set).toHaveBeenCalledTimes(2); + for (const call of redis.set.mock.calls) { + expect(call[2]).toBe("EX"); + expect(call[3]).toBe(ttl); + } + }); + + it("publishInternalPublicStatusConfigSnapshot writes both the versioned key and the current pointer with TTL", async () => { + const mod = await importPublicStatusModule( + "@/lib/public-status/config-snapshot" + ); + const ttl = mod.PUBLIC_STATUS_CONFIG_TTL_SECONDS; + + const redis = { set: vi.fn().mockResolvedValue("OK") }; + + await mod.publishInternalPublicStatusConfigSnapshot({ + snapshot: { + configVersion: "cfg-3", + generatedAt: new Date().toISOString(), + siteTitle: "Test", + siteDescription: "Test", + timeZone: null, + defaultIntervalMinutes: 5, + defaultRangeHours: 24, + groups: [], + }, + redis, + }); + + expect(redis.set).toHaveBeenCalledTimes(2); + for (const call of redis.set.mock.calls) { + expect(call[2]).toBe("EX"); + expect(call[3]).toBe(ttl); + } + }); + + it("publishCurrentPublicStatusConfigPointers (eval path) passes the TTL through Lua ARGV", async () => { + const mod = await importPublicStatusModule( + "@/lib/public-status/config-snapshot" + ); + const ttl = mod.PUBLIC_STATUS_CONFIG_TTL_SECONDS; + + const evalMock = vi.fn().mockResolvedValue(1); + const redis = { + set: vi.fn().mockResolvedValue("OK"), + eval: evalMock, + }; + + await expect( + mod.publishCurrentPublicStatusConfigPointers({ configVersion: "cfg-99", redis }) + ).resolves.toBe(true); + + expect(evalMock).toHaveBeenCalledTimes(1); + const evalArgs = evalMock.mock.calls[0]; + const luaScript = evalArgs[0] as string; + // Lua MUST apply EX atomically with the SET so the pointer + // refreshes its expiration on every successful publish. + expect(luaScript).toMatch(/SET.+'EX'.+ARGV\[2\]/); + // ARGV: [configVersion, ttlSeconds] — both passed as strings. + expect(evalArgs[3]).toBe("cfg-99"); + expect(evalArgs[4]).toBe(String(ttl)); + }); + + it("publishCurrentPublicStatusConfigPointers (non-eval fallback) applies TTL on the bare set", async () => { + const mod = await importPublicStatusModule( + "@/lib/public-status/config-snapshot" + ); + const ttl = mod.PUBLIC_STATUS_CONFIG_TTL_SECONDS; + + const redis: { + set: ReturnType; + get: ReturnType; + } = { + set: vi.fn().mockResolvedValue("OK"), + get: vi.fn().mockResolvedValue(null), + }; + + await expect( + mod.publishCurrentPublicStatusConfigPointers({ configVersion: "cfg-100", redis }) + ).resolves.toBe(true); + + expect(redis.set).toHaveBeenCalledTimes(1); + const setArgs = redis.set.mock.calls[0]; + expect(setArgs[2]).toBe("EX"); + expect(setArgs[3]).toBe(ttl); + }); + }); }); From c983b6402a112b17f3ee775e9fe248f9b72a4bf7 Mon Sep 17 00:00:00 2001 From: ding113 Date: Fri, 15 May 2026 00:47:00 +0800 Subject: [PATCH 5/6] fix(auth,models): i18n new auth error messages + cover models auth outcomes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit and CI flagged two remaining gaps in the previous fix: - The new 401 messages for invalid_api_key / key_disabled / key_expired were hardcoded Chinese strings, ignoring the project's i18n guideline (5 locales: zh-CN, zh-TW, en, ja, ru). The pre-existing strings in the same files (e.g. user_disabled, user_expired) were already hardcoded before this PR and remain so — see PR scope note — but the new branches should follow the established pattern. - /v1/models had no unit coverage for the new key_disabled / key_expired branches in handleAvailableModels.authenticateRequest. A regression back to a generic invalid_api_key would have gone undetected. Add PROXY_INVALID_API_KEY, PROXY_API_KEY_DISABLED, PROXY_API_KEY_EXPIRED codes to AUTH_ERRORS plus translations across all 5 locales. Wire auth-guard.ts and available-models.ts through getErrorMessageServer + next-intl/server's getLocale — same pattern the neighbouring rate-limit-guard.ts already uses. Add tests/unit/models/available-models-auth-outcome.test.ts covering all five 401 branches (key_disabled, key_expired, not_found, user_disabled, user_expired). Existing auth-guard tests mock next-intl/server + getErrorMessageServer so the unit tests can run outside a Next.js request context, and message assertions pin on the ERROR_CODES key (the localized text lives in messages//errors.json). Co-Authored-By: Claude Opus 4.7 (1M context) --- messages/en/errors.json | 3 + messages/ja/errors.json | 3 + messages/ru/errors.json | 3 + messages/zh-CN/errors.json | 3 + messages/zh-TW/errors.json | 3 + src/app/v1/_lib/models/available-models.ts | 33 +++- src/app/v1/_lib/proxy/auth-guard.ts | 16 +- src/lib/utils/error-messages.ts | 4 + .../available-models-auth-outcome.test.ts | 181 ++++++++++++++++++ .../proxy/auth-guard-account-state.test.ts | 26 ++- tests/unit/proxy/auth-guard-precheck.test.ts | 17 ++ 11 files changed, 283 insertions(+), 9 deletions(-) create mode 100644 tests/unit/models/available-models-auth-outcome.test.ts diff --git a/messages/en/errors.json b/messages/en/errors.json index da374c2b3..0e9d3a4ac 100644 --- a/messages/en/errors.json +++ b/messages/en/errors.json @@ -30,6 +30,9 @@ "PERMISSION_DENIED": "Permission denied", "TOKEN_REQUIRED": "Authentication token required", "INVALID_TOKEN": "Invalid authentication token", + "PROXY_INVALID_API_KEY": "Invalid API key. The provided key does not exist or has been deleted.", + "PROXY_API_KEY_DISABLED": "This API key has been disabled. Please contact your administrator to re-enable it, or use a different key.", + "PROXY_API_KEY_EXPIRED": "This API key has expired. Please contact your administrator to renew it or rotate to a new key.", "INTERNAL_ERROR": "Internal server error, please try again later", "DATABASE_ERROR": "Database error", diff --git a/messages/ja/errors.json b/messages/ja/errors.json index 5758e6fc5..b951cd5bb 100644 --- a/messages/ja/errors.json +++ b/messages/ja/errors.json @@ -30,6 +30,9 @@ "PERMISSION_DENIED": "アクセス権限がありません", "TOKEN_REQUIRED": "認証トークンが必要です", "INVALID_TOKEN": "無効な認証トークン", + "PROXY_INVALID_API_KEY": "API キーが無効です。指定されたキーは存在しないか、削除されています。", + "PROXY_API_KEY_DISABLED": "この API キーは無効化されています。管理者に再有効化を依頼するか、別のキーをご使用ください。", + "PROXY_API_KEY_EXPIRED": "この API キーは期限切れです。管理者に更新を依頼するか、新しいキーへ切り替えてください。", "INTERNAL_ERROR": "内部サーバーエラー、後でもう一度お試しください", "DATABASE_ERROR": "データベースエラー", diff --git a/messages/ru/errors.json b/messages/ru/errors.json index 7e4f2502a..87562a3ef 100644 --- a/messages/ru/errors.json +++ b/messages/ru/errors.json @@ -30,6 +30,9 @@ "PERMISSION_DENIED": "Доступ запрещен", "TOKEN_REQUIRED": "Требуется токен аутентификации", "INVALID_TOKEN": "Недействительный токен аутентификации", + "PROXY_INVALID_API_KEY": "Неверный API-ключ. Указанный ключ не существует или был удалён.", + "PROXY_API_KEY_DISABLED": "Этот API-ключ отключён. Обратитесь к администратору, чтобы повторно включить его, или используйте другой ключ.", + "PROXY_API_KEY_EXPIRED": "Срок действия этого API-ключа истёк. Обратитесь к администратору, чтобы продлить срок, или замените ключ.", "INTERNAL_ERROR": "Внутренняя ошибка сервера, попробуйте позже", "DATABASE_ERROR": "Ошибка базы данных", diff --git a/messages/zh-CN/errors.json b/messages/zh-CN/errors.json index 42dc25433..b8d037487 100644 --- a/messages/zh-CN/errors.json +++ b/messages/zh-CN/errors.json @@ -30,6 +30,9 @@ "PERMISSION_DENIED": "权限不足", "TOKEN_REQUIRED": "需要提供认证令牌", "INVALID_TOKEN": "无效的认证令牌", + "PROXY_INVALID_API_KEY": "API 密钥无效。提供的密钥不存在或已被删除。", + "PROXY_API_KEY_DISABLED": "API 密钥已被禁用。请联系管理员重新启用，或使用其他可用密钥。", + "PROXY_API_KEY_EXPIRED": "API 密钥已过期。请联系管理员续期或更换密钥。", "INTERNAL_ERROR": "系统内部错误，请稍后重试", "DATABASE_ERROR": "数据库错误", diff --git a/messages/zh-TW/errors.json b/messages/zh-TW/errors.json index 538e56655..07d104c72 100644 --- a/messages/zh-TW/errors.json +++ b/messages/zh-TW/errors.json @@ -30,6 +30,9 @@ "PERMISSION_DENIED": "權限不足", "TOKEN_REQUIRED": "需要提供認證令牌", "INVALID_TOKEN": "無效的認證令牌", + "PROXY_INVALID_API_KEY": "API 金鑰無效。提供的金鑰不存在或已被刪除。", + "PROXY_API_KEY_DISABLED": "API 金鑰已被停用。請聯絡管理員重新啟用，或使用其他可用金鑰。", + "PROXY_API_KEY_EXPIRED": "API 金鑰已過期。請聯絡管理員續期或更換金鑰。", "INTERNAL_ERROR": "系統內部錯誤，請稍後重試", "DATABASE_ERROR": "資料庫錯誤", diff --git a/src/app/v1/_lib/models/available-models.ts b/src/app/v1/_lib/models/available-models.ts index c09eebcb6..2c62196ea 100644 --- a/src/app/v1/_lib/models/available-models.ts +++ b/src/app/v1/_lib/models/available-models.ts @@ -3,6 +3,7 @@ import { request as undiciRequest } from "undici"; import { normalizeAllowedModelRules } from "@/lib/allowed-model-rules"; import { logger } from "@/lib/logger"; import { createProxyAgentForProvider } from "@/lib/proxy-agent"; +import { ERROR_CODES, getErrorMessageServer } from "@/lib/utils/error-messages"; import { isProviderActiveNow } from "@/lib/utils/provider-schedule"; import { resolveSystemTimezone } from "@/lib/utils/timezone"; import { resolveApiKeyAuthOutcome } from "@/repository/key"; @@ -65,13 +66,39 @@ async function authenticateRequest(c: Context): Promise<{ // Exhaustive switch: see auth-guard.ts for rationale. Adding a new // ApiKeyAuthFailureReason will produce a TypeScript error on the // exhaustiveness fallthrough until this branch is handled explicitly. + const { getLocale } = await import("next-intl/server"); + const locale = await getLocale(); switch (outcome.reason) { case "key_disabled": - throw c.json({ error: { message: "API 密钥已被禁用", type: "key_disabled" } }, 401); + throw c.json( + { + error: { + message: await getErrorMessageServer(locale, ERROR_CODES.PROXY_API_KEY_DISABLED), + type: "key_disabled", + }, + }, + 401 + ); case "key_expired": - throw c.json({ error: { message: "API 密钥已过期", type: "key_expired" } }, 401); + throw c.json( + { + error: { + message: await getErrorMessageServer(locale, ERROR_CODES.PROXY_API_KEY_EXPIRED), + type: "key_expired", + }, + }, + 401 + ); case "not_found": - throw c.json({ error: { message: "API 密钥无效", type: "invalid_api_key" } }, 401); + throw c.json( + { + error: { + message: await getErrorMessageServer(locale, ERROR_CODES.PROXY_INVALID_API_KEY), + type: "invalid_api_key", + }, + }, + 401 + ); default: { const _exhaustive: never = outcome.reason; throw new Error(`Unhandled auth outcome reason: ${JSON.stringify(_exhaustive)}`); diff --git a/src/app/v1/_lib/proxy/auth-guard.ts b/src/app/v1/_lib/proxy/auth-guard.ts index 02d1f858a..6698c2c06 100644 --- a/src/app/v1/_lib/proxy/auth-guard.ts +++ b/src/app/v1/_lib/proxy/auth-guard.ts @@ -2,12 +2,21 @@ import { extractApiKeyFromHeaders as sharedExtractApiKeyFromHeaders } from "@/li import { getClientIpWithFreshSettings } from "@/lib/ip"; import { logger } from "@/lib/logger"; import { LoginAbusePolicy } from "@/lib/security/login-abuse-policy"; +import { ERROR_CODES, getErrorMessageServer } from "@/lib/utils/error-messages"; import { resolveApiKeyAuthOutcome } from "@/repository/key"; import { markUserExpired } from "@/repository/user"; import { GEMINI_PROTOCOL } from "../gemini/protocol"; import { ProxyResponses } from "./responses"; import type { AuthFailureKind, AuthState, ProxySession } from "./session"; +async function getRequestLocale(): Promise { + // Match the rate-limit-guard pattern: read next-intl's request locale + // (Accept-Language or cookie) lazily inside the guard so the proxy hot + // path does not pay the import cost on every cold start. + const { getLocale } = await import("next-intl/server"); + return await getLocale(); +} + /** * Build an auth failure AuthState. The factory exists so every call site is * forced (by the function signature) to supply both `failureKind` and an @@ -199,6 +208,7 @@ export class ProxyAuthenticator { // repository layer will trigger a compile error in assertNever below // until this guard is updated, preventing a new variant from silently // falling into the wrong branch. + const locale = await getRequestLocale(); switch (outcome.reason) { case "not_found": logger.debug("[ProxyAuthenticator] API key validation failed: not found", { @@ -212,7 +222,7 @@ export class ProxyAuthenticator { failureKind: "credentials", errorResponse: ProxyResponses.buildError( 401, - "API 密钥无效。提供的密钥不存在或已被删除。", + await getErrorMessageServer(locale, ERROR_CODES.PROXY_INVALID_API_KEY), "invalid_api_key" ), }); @@ -226,7 +236,7 @@ export class ProxyAuthenticator { failureKind: "account_state", errorResponse: ProxyResponses.buildError( 401, - "API 密钥已被禁用。请联系管理员重新启用，或使用其他可用密钥。", + await getErrorMessageServer(locale, ERROR_CODES.PROXY_API_KEY_DISABLED), "key_disabled" ), }); @@ -240,7 +250,7 @@ export class ProxyAuthenticator { failureKind: "account_state", errorResponse: ProxyResponses.buildError( 401, - "API 密钥已过期。请联系管理员续期或更换密钥。", + await getErrorMessageServer(locale, ERROR_CODES.PROXY_API_KEY_EXPIRED), "key_expired" ), }); diff --git a/src/lib/utils/error-messages.ts b/src/lib/utils/error-messages.ts index c44868025..176141041 100644 --- a/src/lib/utils/error-messages.ts +++ b/src/lib/utils/error-messages.ts @@ -64,6 +64,10 @@ export const AUTH_ERRORS = { PERMISSION_DENIED: "PERMISSION_DENIED", TOKEN_REQUIRED: "TOKEN_REQUIRED", INVALID_TOKEN: "INVALID_TOKEN", + // Proxy-side API key auth outcomes — surfaced to /v1/* and /v1/models clients. + PROXY_INVALID_API_KEY: "PROXY_INVALID_API_KEY", + PROXY_API_KEY_DISABLED: "PROXY_API_KEY_DISABLED", + PROXY_API_KEY_EXPIRED: "PROXY_API_KEY_EXPIRED", } as const; // Server Error Codes diff --git a/tests/unit/models/available-models-auth-outcome.test.ts b/tests/unit/models/available-models-auth-outcome.test.ts new file mode 100644 index 000000000..1be8e287b --- /dev/null +++ b/tests/unit/models/available-models-auth-outcome.test.ts @@ -0,0 +1,181 @@ +/** + * Regression coverage for the `/v1/models` auth chain: every + * `ApiKeyAuthFailureReason` branch and every user-state branch must surface + * the correct 401 `error.type` so a downstream regression back to a generic + * `invalid_api_key` would be caught. + * + * Companion to `tests/unit/proxy/auth-guard-account-state.test.ts`, which + * covers the same matrix on the proxy auth guard. + */ + +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/repository/key", () => ({ + resolveApiKeyAuthOutcome: vi.fn(), +})); + +vi.mock("@/lib/proxy-agent", () => ({ + createProxyAgentForProvider: vi.fn(), +})); + +vi.mock("@/lib/utils/timezone", () => ({ + resolveSystemTimezone: vi.fn().mockResolvedValue("UTC"), +})); + +vi.mock("@/lib/utils/provider-schedule", () => ({ + isProviderActiveNow: vi.fn().mockReturnValue(true), +})); + +vi.mock("@/app/v1/_lib/proxy/provider-selector", () => ({ + checkProviderGroupMatch: vi.fn().mockReturnValue(true), +})); + +vi.mock("@/repository/provider", () => ({ + findAllProviders: vi.fn().mockResolvedValue([]), +})); + +vi.mock("next-intl/server", () => ({ + getLocale: vi.fn().mockResolvedValue("en"), +})); + +vi.mock("@/lib/utils/error-messages", async () => { + const actual = await vi.importActual( + "@/lib/utils/error-messages" + ); + return { + ...actual, + getErrorMessageServer: vi.fn(async (_locale: string, code: string) => code), + }; +}); + +function makeContext(apiKey: string) { + let thrown: Response | null = null; + const ctx = { + req: { + path: "/v1/models", + url: "http://localhost/v1/models", + method: "GET", + header: (name: string) => { + const normalized = name.toLowerCase(); + if (normalized === "x-api-key") return apiKey; + if (normalized === "anthropic-version") return undefined; + return undefined; + }, + query: () => undefined, + }, + json: (body: unknown, status?: number) => { + thrown = new Response(JSON.stringify(body), { + status: status ?? 200, + headers: { "content-type": "application/json" }, + }); + return thrown; + }, + getResponse: () => thrown, + }; + return ctx; +} + +async function callAuthAndCaptureResponse(ctx: ReturnType): Promise { + // handleAvailableModels invokes authenticateRequest internally; both + // resolve through Hono's c.json(...) which the makeContext helper stashes. + // Because the auth helper throws the c.json response, we catch and inspect. + const { handleAvailableModels } = await import("@/app/v1/_lib/models/available-models"); + try { + await handleAvailableModels(ctx as never); + } catch (thrown) { + if (thrown instanceof Response) { + return thrown; + } + throw thrown; + } + // Fallthrough: ctx.json was called via authenticateRequest with throw, so + // the stashed response is what we want. + const response = ctx.getResponse(); + if (!response) { + throw new Error("Expected handleAvailableModels to throw an auth response"); + } + return response; +} + +async function readErrorBody(response: Response) { + const json = (await response.json()) as { error: { message: string; type: string } }; + return json.error; +} + +describe("handleAvailableModels auth outcomes", () => { + it("returns 401 key_disabled for a disabled key", async () => { + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + vi.mocked(resolveApiKeyAuthOutcome).mockResolvedValueOnce({ + ok: false, + reason: "key_disabled", + }); + + const response = await callAuthAndCaptureResponse(makeContext("sk-disabled")); + + expect(response.status).toBe(401); + const error = await readErrorBody(response); + expect(error.type).toBe("key_disabled"); + expect(error.message).toBe("PROXY_API_KEY_DISABLED"); + }); + + it("returns 401 key_expired for an expired key", async () => { + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + vi.mocked(resolveApiKeyAuthOutcome).mockResolvedValueOnce({ + ok: false, + reason: "key_expired", + }); + + const response = await callAuthAndCaptureResponse(makeContext("sk-expired")); + + expect(response.status).toBe(401); + const error = await readErrorBody(response); + expect(error.type).toBe("key_expired"); + expect(error.message).toBe("PROXY_API_KEY_EXPIRED"); + }); + + it("returns 401 invalid_api_key for an unknown key", async () => { + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + vi.mocked(resolveApiKeyAuthOutcome).mockResolvedValueOnce({ + ok: false, + reason: "not_found", + }); + + const response = await callAuthAndCaptureResponse(makeContext("sk-unknown")); + + expect(response.status).toBe(401); + const error = await readErrorBody(response); + expect(error.type).toBe("invalid_api_key"); + expect(error.message).toBe("PROXY_INVALID_API_KEY"); + }); + + it("returns 401 user_disabled when the user account is disabled", async () => { + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + vi.mocked(resolveApiKeyAuthOutcome).mockResolvedValueOnce({ + ok: true, + user: { id: 42, providerGroup: null, isEnabled: false, expiresAt: null }, + key: { providerGroup: null, name: "x" }, + } as never); + + const response = await callAuthAndCaptureResponse(makeContext("sk-userdisabled")); + + expect(response.status).toBe(401); + const error = await readErrorBody(response); + expect(error.type).toBe("user_disabled"); + }); + + it("returns 401 user_expired when the user account is expired", async () => { + const { resolveApiKeyAuthOutcome } = await import("@/repository/key"); + const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000); + vi.mocked(resolveApiKeyAuthOutcome).mockResolvedValueOnce({ + ok: true, + user: { id: 43, providerGroup: null, isEnabled: true, expiresAt: yesterday }, + key: { providerGroup: null, name: "x" }, + } as never); + + const response = await callAuthAndCaptureResponse(makeContext("sk-userexpired")); + + expect(response.status).toBe(401); + const error = await readErrorBody(response); + expect(error.type).toBe("user_expired"); + }); +}); diff --git a/tests/unit/proxy/auth-guard-account-state.test.ts b/tests/unit/proxy/auth-guard-account-state.test.ts index 2a3c56c1a..2f94b5704 100644 --- a/tests/unit/proxy/auth-guard-account-state.test.ts +++ b/tests/unit/proxy/auth-guard-account-state.test.ts @@ -41,6 +41,24 @@ vi.mock("@/lib/security/login-abuse-policy", () => ({ }, })); +// Bypass next-intl's request-context lookup so the proxy guard can run +// outside a real Next.js request. The mocked helper returns the i18n code +// itself so message assertions can pin on the key, not on translation +// content (which lives in messages//errors.json). +vi.mock("next-intl/server", () => ({ + getLocale: vi.fn().mockResolvedValue("en"), +})); + +vi.mock("@/lib/utils/error-messages", async () => { + const actual = await vi.importActual( + "@/lib/utils/error-messages" + ); + return { + ...actual, + getErrorMessageServer: vi.fn(async (_locale: string, code: string) => code), + }; +}); + function makeSession(ip: string, apiKey: string) { return { headers: new Headers({ @@ -88,7 +106,9 @@ describe("ProxyAuthenticator account-state failures", () => { const error = await readErrorBody(response as Response); expect(error.type).toBe("key_disabled"); expect(error.code).toBe("key_disabled"); - expect(error.message).toMatch(/已被禁用/); + // Message is the i18n key (mocked); the actual localized text lives in + // messages//errors.json under this key. + expect(error.message).toBe("PROXY_API_KEY_DISABLED"); expect(policyRecordFailure).not.toHaveBeenCalled(); expect(policyRecordSuccess).not.toHaveBeenCalled(); @@ -106,7 +126,7 @@ describe("ProxyAuthenticator account-state failures", () => { const error = await readErrorBody(response as Response); expect(error.type).toBe("key_expired"); expect(error.code).toBe("key_expired"); - expect(error.message).toMatch(/已过期/); + expect(error.message).toBe("PROXY_API_KEY_EXPIRED"); expect(policyRecordFailure).not.toHaveBeenCalled(); }); @@ -164,7 +184,7 @@ describe("ProxyAuthenticator account-state failures", () => { const error = await readErrorBody(response as Response); expect(error.type).toBe("invalid_api_key"); - expect(error.message).toMatch(/不存在|已被删除/); + expect(error.message).toBe("PROXY_INVALID_API_KEY"); expect(policyRecordFailure).toHaveBeenCalledWith("203.0.113.24", "sk-doesnotexist"); }); diff --git a/tests/unit/proxy/auth-guard-precheck.test.ts b/tests/unit/proxy/auth-guard-precheck.test.ts index f9b938a60..b1d2278f2 100644 --- a/tests/unit/proxy/auth-guard-precheck.test.ts +++ b/tests/unit/proxy/auth-guard-precheck.test.ts @@ -29,6 +29,23 @@ vi.mock("@/lib/security/login-abuse-policy", () => ({ }, })); +// The auth guard now looks up the request locale to localize 401 messages +// (see ERROR_CODES.PROXY_*). Mock both helpers so unit tests can run outside +// a Next.js request context. +vi.mock("next-intl/server", () => ({ + getLocale: vi.fn().mockResolvedValue("en"), +})); + +vi.mock("@/lib/utils/error-messages", async () => { + const actual = await vi.importActual( + "@/lib/utils/error-messages" + ); + return { + ...actual, + getErrorMessageServer: vi.fn(async (_locale: string, code: string) => code), + }; +}); + function makeSession(ip: string, apiKey: string) { return { headers: new Headers({ From 5cd75941a4d6f1ab8bc86fcf955ce65c9d713c7b Mon Sep 17 00:00:00 2001 From: ding113 Date: Fri, 15 May 2026 01:04:07 +0800 Subject: [PATCH 6/6] fix(public-status): drop TTL from current-pointer Redis keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit greptile flagged a P1: applying the same 30-day TTL to the three "current pointer" keys would dark out any deployment that goes longer than the TTL without publishing a new config. Pointer keys don't accumulate — only one entry per pointer name exists, overwritten atomically on every publish — so they MUST persist until explicitly overwritten. Only the versioned snapshot keys (`public-status:v1:config:` and `:config-internal:`) keep the 30-day TTL — those are the ones that accumulate as new config versions are minted. The pointer publisher's Lua script and JS fallback now write bare `SET` without `EX`. Tests assert the split: the versioned write carries `EX ` and the pointer write is a bare two-arg `set(key, value)` with no TTL. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/public-status/config-snapshot.ts | 53 +++++++++-------- .../public-status/config-snapshot.test.ts | 57 ++++++++++++------- 2 files changed, 63 insertions(+), 47 deletions(-) diff --git a/src/lib/public-status/config-snapshot.ts b/src/lib/public-status/config-snapshot.ts index 4fe313d8a..c40b9736a 100644 --- a/src/lib/public-status/config-snapshot.ts +++ b/src/lib/public-status/config-snapshot.ts @@ -9,17 +9,23 @@ import { export const DEFAULT_PUBLIC_STATUS_SITE_DESCRIPTION = "Request-derived public status"; /** - * TTL (seconds) applied to every Redis key written by this module. + * TTL (seconds) applied to *versioned* config snapshot keys written by this + * module — i.e. `public-status:v1:config:` and the internal variant. * * 30 days matches `GENERATION_PROJECTION_TTL_SECONDS` in `rebuild-worker.ts`, * which already governs the manifest / series / snapshot keys for this - * feature. Versioned config snapshot keys - * (`public-status:v1:config:` and the internal variant) are written - * once per config publication; without a TTL they accumulate indefinitely as - * provider/group/system-settings changes mint new config versions. Each - * publish refreshes the TTL on the "current" pointer keys, so as long as - * configurations are published at least every 30 days the live pointers - * never expire while stale versioned snapshots get cleaned up naturally. + * feature. These keys accumulate forever otherwise, since every config + * publication mints a new version (provider/group/system-settings changes). + * + * The three *current-pointer* keys + * (`buildPublicStatusConfigSnapshotKey()` with no argument, + * `buildPublicStatusInternalConfigSnapshotKey()` with no argument, and + * `buildPublicStatusConfigVersionPointerKey()`) are intentionally written + * WITHOUT a TTL — they always reference the latest config and are + * overwritten atomically on every publish, so they never accumulate. Giving + * them a TTL would dark out the public status page on any deployment that + * goes longer than the TTL without publishing (e.g. an idle staging env or + * a stable production whose config does not change for a month). */ export const PUBLIC_STATUS_CONFIG_TTL_SECONDS = 60 * 60 * 24 * 30; @@ -224,13 +230,14 @@ export async function publishPublicStatusConfigSnapshot(input: { const redis = input.redis ?? getRedisClient({ allowWhenRateLimitDisabled: true }); if (redis) { + // Versioned snapshot key: TTL'd so old versions get cleaned up. await redis.set(key, JSON.stringify(snapshot), "EX", PUBLIC_STATUS_CONFIG_TTL_SECONDS); if (input.setCurrentPointer !== false) { + // Current-pointer key: no TTL. It is overwritten on every publish and + // is the only entry-point the read path uses — see TTL constant above. await redis.set( buildPublicStatusConfigSnapshotKey(), - JSON.stringify({ key, configVersion: snapshot.configVersion }), - "EX", - PUBLIC_STATUS_CONFIG_TTL_SECONDS + JSON.stringify({ key, configVersion: snapshot.configVersion }) ); } } @@ -263,13 +270,13 @@ export async function publishInternalPublicStatusConfigSnapshot(input: { const redis = input.redis ?? getRedisClient({ allowWhenRateLimitDisabled: true }); if (redis) { + // Versioned snapshot key: TTL'd so old versions get cleaned up. await redis.set(key, JSON.stringify(input.snapshot), "EX", PUBLIC_STATUS_CONFIG_TTL_SECONDS); if (input.setCurrentPointer !== false) { + // Current-pointer key: no TTL — see TTL constant rationale above. await redis.set( buildPublicStatusInternalConfigSnapshotKey(), - JSON.stringify({ key, configVersion: input.snapshot.configVersion }), - "EX", - PUBLIC_STATUS_CONFIG_TTL_SECONDS + JSON.stringify({ key, configVersion: input.snapshot.configVersion }) ); } } @@ -291,24 +298,20 @@ export async function publishCurrentPublicStatusConfigPointers(input: { } const pointerKey = buildPublicStatusConfigVersionPointerKey(); + // This is a current-pointer key: do NOT apply a TTL. It is overwritten on + // every successful publish; an idle deployment with no config change for + // longer than any TTL we would pick would otherwise lose the pointer and + // dark out the public status page. if (typeof redis.eval === "function") { - // SET ... EX applies the TTL atomically with the write, refreshing - // the expiration on every successful publish. const luaScript = ` local current = redis.call('GET', KEYS[1]) if (not current) or current <= ARGV[1] then - redis.call('SET', KEYS[1], ARGV[1], 'EX', ARGV[2]) + redis.call('SET', KEYS[1], ARGV[1]) return 1 end return 0 `; - const result = await redis.eval( - luaScript, - 1, - pointerKey, - input.configVersion, - String(PUBLIC_STATUS_CONFIG_TTL_SECONDS) - ); + const result = await redis.eval(luaScript, 1, pointerKey, input.configVersion); return result === 1; } @@ -320,7 +323,7 @@ export async function publishCurrentPublicStatusConfigPointers(input: { return false; } - await redis.set(pointerKey, input.configVersion, "EX", PUBLIC_STATUS_CONFIG_TTL_SECONDS); + await redis.set(pointerKey, input.configVersion); return true; } diff --git a/tests/unit/public-status/config-snapshot.test.ts b/tests/unit/public-status/config-snapshot.test.ts index 3b896f1aa..ea2bcc7b3 100644 --- a/tests/unit/public-status/config-snapshot.test.ts +++ b/tests/unit/public-status/config-snapshot.test.ts @@ -189,8 +189,15 @@ describe("public-status config snapshot", () => { // Regression: before this guard, every config publish wrote Redis keys // without TTL, so versioned snapshot keys accumulated forever as // provider/group/system-settings changes minted new config versions. - describe("Redis writes apply PUBLIC_STATUS_CONFIG_TTL_SECONDS", () => { - it("publishPublicStatusConfigSnapshot writes both the versioned key and the current pointer with TTL", async () => { + describe("Redis TTL strategy: only versioned snapshot keys get a TTL", () => { + // Versioned keys (`public-status:v1:config:` and the internal + // variant) accumulate forever as configs are republished and MUST expire. + // The three "current pointer" keys are overwritten on every publish, so + // they MUST NOT carry a TTL — otherwise an idle deployment that goes + // longer than the TTL without a config change would lose its pointer and + // the public status page would silently go dark. + + it("publishPublicStatusConfigSnapshot TTLs the versioned key but leaves the pointer untouched", async () => { const mod = await importPublicStatusModule( "@/lib/public-status/config-snapshot" ); @@ -215,13 +222,18 @@ describe("public-status config snapshot", () => { }); expect(redis.set).toHaveBeenCalledTimes(2); - for (const call of redis.set.mock.calls) { - expect(call[2]).toBe("EX"); - expect(call[3]).toBe(ttl); - } + // First call: versioned key — MUST have EX TTL. + const [versionedKey, , versionedMode, versionedTtl] = redis.set.mock.calls[0]; + expect(versionedKey).toMatch(/:config:cfg-2$/); + expect(versionedMode).toBe("EX"); + expect(versionedTtl).toBe(ttl); + // Second call: current pointer key — MUST be bare set with no TTL. + const pointerCall = redis.set.mock.calls[1]; + expect(pointerCall).toHaveLength(2); + expect(pointerCall[0]).toMatch(/:config:current$/); }); - it("publishInternalPublicStatusConfigSnapshot writes both the versioned key and the current pointer with TTL", async () => { + it("publishInternalPublicStatusConfigSnapshot TTLs the versioned key but leaves the pointer untouched", async () => { const mod = await importPublicStatusModule( "@/lib/public-status/config-snapshot" ); @@ -244,17 +256,19 @@ describe("public-status config snapshot", () => { }); expect(redis.set).toHaveBeenCalledTimes(2); - for (const call of redis.set.mock.calls) { - expect(call[2]).toBe("EX"); - expect(call[3]).toBe(ttl); - } + const [versionedKey, , versionedMode, versionedTtl] = redis.set.mock.calls[0]; + expect(versionedKey).toMatch(/:config-internal:cfg-3$/); + expect(versionedMode).toBe("EX"); + expect(versionedTtl).toBe(ttl); + const pointerCall = redis.set.mock.calls[1]; + expect(pointerCall).toHaveLength(2); + expect(pointerCall[0]).toMatch(/:config-internal:current$/); }); - it("publishCurrentPublicStatusConfigPointers (eval path) passes the TTL through Lua ARGV", async () => { + it("publishCurrentPublicStatusConfigPointers (eval path) writes the pointer without TTL", async () => { const mod = await importPublicStatusModule( "@/lib/public-status/config-snapshot" ); - const ttl = mod.PUBLIC_STATUS_CONFIG_TTL_SECONDS; const evalMock = vi.fn().mockResolvedValue(1); const redis = { @@ -269,19 +283,17 @@ describe("public-status config snapshot", () => { expect(evalMock).toHaveBeenCalledTimes(1); const evalArgs = evalMock.mock.calls[0]; const luaScript = evalArgs[0] as string; - // Lua MUST apply EX atomically with the SET so the pointer - // refreshes its expiration on every successful publish. - expect(luaScript).toMatch(/SET.+'EX'.+ARGV\[2\]/); - // ARGV: [configVersion, ttlSeconds] — both passed as strings. + // Pointer Lua MUST NOT apply an EX TTL — see TTL strategy above. + expect(luaScript).not.toMatch(/EX/); + // ARGV: [configVersion] only. expect(evalArgs[3]).toBe("cfg-99"); - expect(evalArgs[4]).toBe(String(ttl)); + expect(evalArgs[4]).toBeUndefined(); }); - it("publishCurrentPublicStatusConfigPointers (non-eval fallback) applies TTL on the bare set", async () => { + it("publishCurrentPublicStatusConfigPointers (non-eval fallback) writes the pointer without TTL", async () => { const mod = await importPublicStatusModule( "@/lib/public-status/config-snapshot" ); - const ttl = mod.PUBLIC_STATUS_CONFIG_TTL_SECONDS; const redis: { set: ReturnType; @@ -297,8 +309,9 @@ describe("public-status config snapshot", () => { expect(redis.set).toHaveBeenCalledTimes(1); const setArgs = redis.set.mock.calls[0]; - expect(setArgs[2]).toBe("EX"); - expect(setArgs[3]).toBe(ttl); + // Bare two-arg set — no EX, no TTL. + expect(setArgs).toHaveLength(2); + expect(setArgs[1]).toBe("cfg-100"); }); }); });