fix(auth): check model lockout before returning synthetic noauth connection (#13547)

No-auth providers now honor a recorded model-only lockout before `getProviderCredentials` hands back the synthetic `noauth` connection (#13483). That early return skipped the per-connection status pass, so a `model_capacity` lockout was recorded but never enforced, and every request re-tried the locked model for a wasted upstream round-trip before failing over.

Maintainer additions: carried your `tests/unit/noauth-model-lockout.test.ts` from #13527. 3 of its 4 cases fail on the release tip without the fix and pass with it. Rebaselined `src/sse/services/auth.ts` 3542→3556 in `file-size-baseline.json` with a dated annotation.

Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green.

Thanks @KooshaPari!
This commit is contained in:
Koosha Paridehpour
2026-09-14 19:28:25 -07:00
committed by GitHub
parent 97ae10179c
commit bf6658984b
3 changed files with 101 additions and 1 deletions

View File

@@ -1,4 +1,5 @@
{
"_rebaseline_2026_09_14_13547_noauth_model_lockout": "PR #13547 own growth: src/sse/services/auth.ts 3542->3556 (+14). The synthetic noauth connection short-circuited before the per-connection status pass, so a recorded model-only lockout was never enforced and the locked model was retried on every request. Irreducible at the early-return site. Covered by tests/unit/noauth-model-lockout.test.ts (carried from #13527).",
"_rebaseline_2026_09_14_13404_healthcheck_backup_prune": "PR #13404 own growth: src/lib/db/core.ts 1745->1767 (+22). createManagedDbBackup (the health-check-repair snapshot path) never ran retention, so every restart of a healthy DB added a full-size copy to db_backups/; it now prunes with the same env-driven limits as backup.ts, importing backupRetention directly to avoid the backup.ts cycle. Covered by tests/unit/db-backup-healthcheck-prune-13308.test.ts.",
"_rebaseline_2026_09_14_13349_virtualfactory_custom_models_guard": "PR #13349 own growth: open-sse/services/autoCombo/virtualFactory.ts 1219->1230 (+11). The customModels key_value blob is operator-writable raw JSON, so a null or non-object row null-derefed every read and no auto/* pool could materialize; the builder now filters rows the same way catalog.ts already does. Irreducible at the read site. Covered by tests/unit/combo-auto-pool-visible-only.test.ts.",
"_rebaseline_2026_09_11_12732_catalog_timeout_pin": "+1 in tests/unit/models-catalog-route.test.ts (1652->1653) for a single line: process.env.CATALOG_BUILD_TIMEOUT_MS. #12627 bounds a cold catalog build at 8s; beforeEach resets the catalog cache so every case in this file pays a cold build, and a tsx runner needs 10-13s under load — the file returned catalog_build_timeout instead of rows and oscillated between 1 and 10 failures per run, reddening the whole PR queue (base-red #12732). The bound itself stays covered by tests/unit/12627-catalog-inflight-timeout.test.ts. The file is already at its frozen ceiling, so the pin cannot be absorbed; structural shrink tracked in #3501.",
@@ -477,7 +478,7 @@
"src/shared/constants/providers/apikey/gateways.ts": 1502,
"src/shared/services/cliRuntime.ts": 1296,
"src/sse/handlers/chat.ts": 2462,
"src/sse/services/auth.ts": 3542,
"src/sse/services/auth.ts": 3556,
"tests/unit/account-fallback-service.test.ts": 2453,
"tests/unit/provider-validation-specialty.test.ts": 4656,
"open-sse/services/autoCombo/virtualFactory.ts": 1230,

View File

@@ -1225,6 +1225,20 @@ export async function getProviderCredentials(
// respected (the no-auth provider will be rejected if it has no real connections
// matching the allowlist, or a real connection row will be selected if present).
if (!allowedConnections || allowedConnections.length === 0) {
// #13483: check model-only lockout before handing back the synthetic
// connection. Without this, a locked model (e.g. 400 model_capacity)
// is retried on every request because the noauth path short-circuits
// before the per-connection status pass that classifies modelLocked.
const modelLockout = requestedModel
? getModelLockoutInfo(resolvedId, SYNTHETIC_NOAUTH_CONNECTION_ID, requestedModel)
: null;
if (modelLockout && modelLockout.remainingMs > 0) {
log.debug(
"AUTH",
`${resolvedId} | noauth model-only lockout for ${requestedModel}${modelLockout.remainingMs}ms remaining, returning null`
);
return null;
}
return await maybeSyntheticNoAuthFallback(resolvedId, excludedForNoAuth);
}
}

View File

@@ -0,0 +1,85 @@
/**
* Tests for #13483: model-only lockout must be enforced for no-auth providers.
*
* Before the fix, no-auth providers (opencode, duckduckgo-web, etc.) returned
* synthetic "noauth" credentials early in getProviderCredentials, bypassing the
* model lockout check. A model_capacity lockout was recorded but never enforced
* — every request retried the same locked model, paying a wasted upstream
* round-trip (~2s) before failing over.
*/
import { test, after } from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import path from "node:path";
import os from "node:os";
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-noauth-lockout-"));
process.env.DATA_DIR = TEST_DATA_DIR;
process.env.API_KEY_SECRET = "test-noauth-lockout-secret";
const core = await import("../../src/lib/db/core.ts");
const auth = await import("../../src/sse/services/auth.ts");
const { recordModelLockoutFailure, clearAllModelLockouts } =
await import("../../open-sse/services/accountFallback.ts");
after(() => {
clearAllModelLockouts();
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
});
test("#13483: noauth provider returns null when model is lockout-blocked", async () => {
const provider = "opencode";
const model = "deepseek-v4-flash-free";
// Record a model lockout for the synthetic "noauth" connection
recordModelLockoutFailure(provider, "noauth", model, "model_capacity", 400, 1800_000);
// getProviderCredentials should return null because the model is locked
const result = await auth.getProviderCredentials(provider, null, null, model);
assert.equal(result, null, "noauth provider should return null when model is lockout-blocked");
});
test("#13483: noauth provider still works when model is NOT lockout-blocked", async () => {
clearAllModelLockouts();
const provider = "opencode";
const model = "some-other-model";
// No lockout recorded — should return synthetic credentials
const result = await auth.getProviderCredentials(provider, null, null, model);
assert.ok(result !== null, "noauth provider should return credentials when model is not locked");
});
test("#13483: noauth lockout does not block a different model", async () => {
clearAllModelLockouts();
const provider = "opencode";
const lockedModel = "deepseek-v4-flash-free";
const otherModel = "kimi-latest";
// Lock only one model
recordModelLockoutFailure(provider, "noauth", lockedModel, "model_capacity", 400, 1800_000);
// The locked model should be blocked
const result1 = await auth.getProviderCredentials(provider, null, null, lockedModel);
assert.equal(result1, null, "locked model should return null");
// A different model should NOT be blocked
const result2 = await auth.getProviderCredentials(provider, null, null, otherModel);
assert.ok(result2 !== null, "different model should still get credentials");
});
test("#13483: noauth lockout does not affect non-noauth providers", async () => {
clearAllModelLockouts();
const model = "gpt-4";
// Record lockout for a noauth provider
recordModelLockoutFailure("opencode", "noauth", model, "model_capacity", 400, 1800_000);
// openai is NOT a noauth provider — it should not be affected by this check
// (openai has its own connection-based lockout path; this test just verifies
// the noauth early-return path doesn't leak lockouts to other providers)
// We can't easily test openai here without DB connections, but we verify
// the opencode noauth path specifically.
const result = await auth.getProviderCredentials("opencode", null, null, model);
assert.equal(result, null, "opencode noauth should respect its own lockout");
});