fix(quota): policy inválida não vaza allow + guard connectionIds vazio [Fase 3 #10] (#4901)

Integrated into release/v3.8.36

src/lib/db/quotaPools.ts

@@ -144,11 +144,25 @@ interface AllocationRow {
   policy: string;
 }
 
+const VALID_POLICIES: ReadonlySet<string> = new Set<Policy>(["hard", "soft", "burst"]);
+
+/**
+ * Fail-safe policy normalization at the DB read boundary. A column value outside
+ * `hard | soft | burst` (corrupted/legacy row, or a value inserted while the
+ * table CHECK constraint was bypassed) is coerced to the most restrictive
+ * policy, `hard`, instead of being trusted via a raw `as Policy` cast. This
+ * prevents an unknown policy from reaching the fair-share engine, where it would
+ * otherwise be a silent fail-OPEN (issue #10).
+ */
+function normalizePolicy(value: string): Policy {
+  return VALID_POLICIES.has(value) ? (value as Policy) : "hard";
+}
+
 function rowToAllocation(row: AllocationRow): PoolAllocation {
   const alloc: PoolAllocation = {
     apiKeyId: row.api_key_id,
     weight: row.weight,
-    policy: row.policy as Policy,
+    policy: normalizePolicy(row.policy),
   };
   if (row.cap_value != null) alloc.capValue = row.cap_value;
   if (row.cap_unit != null) alloc.capUnit = row.cap_unit as QuotaUnit;

src/lib/quota/fairShare.ts

@@ -61,6 +61,19 @@ function dimensionKeyString(key: FairShareDimension["key"]): string {
   return `${key.poolId}:${key.unit}:${key.window}`;
 }
 
+const KNOWN_POLICIES: ReadonlySet<Policy> = new Set<Policy>(["hard", "soft", "burst"]);
+
+/**
+ * Fail-safe policy normalization. Any value outside the known
+ * `hard | soft | burst` set (e.g. a corrupted DB row that reached this engine
+ * through an unchecked `row.policy as Policy` cast) is treated as the most
+ * restrictive policy, `hard`. This closes a fail-OPEN hole: an unknown policy
+ * used to fall through every `switch` case and return a silent `allow`.
+ */
+function normalizePolicy(policy: Policy): Policy {
+  return KNOWN_POLICIES.has(policy) ? policy : "hard";
+}
+
 // ---------------------------------------------------------------------------
 // Core algorithm
 // ---------------------------------------------------------------------------
@@ -77,6 +90,10 @@ export function decideFairShare(input: FairShareInput): FairShareDecision {
     return { kind: "allow", reason: "ok" };
   }
 
+  // Fail-safe: an unknown/corrupted policy is treated as `hard` (most
+  // restrictive) so it can never silently bypass fair-share enforcement.
+  const effectivePolicy = normalizePolicy(allocation.policy);
+
   let anyPenalized = false;
 
   for (const dim of dimensions) {
@@ -97,7 +114,7 @@ export function decideFairShare(input: FairShareInput): FairShareDecision {
     // If the pool's global limit is already reached AND this key's request
     // would exceed it (burst mode without borrow room), block as "global-saturated".
     if (dim.consumedTotal >= dim.limit) {
-      if (allocation.policy !== "burst") {
+      if (effectivePolicy !== "burst") {
         return { kind: "block", reason: "global-saturated" };
       }
       // burst also blocked when no room at all
@@ -108,7 +125,9 @@ export function decideFairShare(input: FairShareInput): FairShareDecision {
 
     if (isStrict) {
       // ── Strict mode ────────────────────────────────────────────────────
-      switch (allocation.policy) {
+      // effectivePolicy is normalized (unknown → hard), so these cases are
+      // exhaustive and an unknown policy is enforced as hard.
+      switch (effectivePolicy) {
         case "hard":
           // Hard: block once consumed >= fair_share
           if (consumed >= fairShare) {
@@ -131,7 +150,8 @@ export function decideFairShare(input: FairShareInput): FairShareDecision {
     } else {
       // ── Generous mode ──────────────────────────────────────────────────
       // There is slack — allow borrowing up to the global limit.
-      switch (allocation.policy) {
+      // effectivePolicy is normalized (unknown → hard).
+      switch (effectivePolicy) {
         case "hard":
           // Hard in generous mode: allow if global limit not reached AND
           // the key is within global limit (which we know because

tests/unit/db-quota-pools.test.ts

@@ -20,6 +20,7 @@ process.env.DATA_DIR = TEST_DATA_DIR;
 
 const core = await import("../../src/lib/db/core.ts");
 const poolsDb = await import("../../src/lib/db/quotaPools.ts");
+const { getDbInstance } = core;
 
 async function resetStorage() {
   core.resetDbInstance();
@@ -260,3 +261,61 @@ test("allocation stores optional capValue and capUnit correctly", () => {
   assert.equal(alloc.capValue, 1000);
   assert.equal(alloc.capUnit, "requests");
 });
+
+// ---------------------------------------------------------------------------
+// Guard A (issue #10): corrupted/unknown policy in the DB must be normalized to
+// the most restrictive policy ('hard') at the read boundary — never trusted via
+// `row.policy as Policy`. A garbage policy reaching the fair-share engine would
+// fall through every switch case and silently ALLOW (fail-OPEN).
+//
+// The schema has CHECK (policy IN ('hard','soft','burst')), so we bypass it with
+// PRAGMA ignore_check_constraints to simulate a legacy/corrupted row.
+// ---------------------------------------------------------------------------
+
+test("rowToAllocation normalizes an unknown DB policy to 'hard' (Guard A)", () => {
+  const pool = poolsDb.createPool({
+    connectionId: "c-guardA",
+    name: "Corrupt Policy Pool",
+    allocations: [{ apiKeyId: "k-corrupt", weight: 100, policy: "soft" }],
+  });
+
+  // Inject a corrupted policy directly, bypassing the CHECK constraint.
+  const db = getDbInstance() as unknown as {
+    pragma: (s: string) => unknown;
+    prepare: (sql: string) => { run: (...p: unknown[]) => unknown };
+  };
+  db.pragma("ignore_check_constraints = ON");
+  db.prepare("UPDATE quota_allocations SET policy = ? WHERE pool_id = ? AND api_key_id = ?").run(
+    "bogus-policy",
+    pool.id,
+    "k-corrupt"
+  );
+  db.pragma("ignore_check_constraints = OFF");
+
+  // Read through the domain module — the unknown policy must become 'hard'.
+  const found = poolsDb.getPool(pool.id)!;
+  const alloc = found.allocations.find((a) => a.apiKeyId === "k-corrupt")!;
+  assert.equal(alloc.policy, "hard", "unknown DB policy must be normalized to 'hard'");
+
+  // Same expectation via listAllocationsForApiKey (the other read path).
+  const list = poolsDb.listAllocationsForApiKey("k-corrupt");
+  assert.equal(list.length, 1);
+  assert.equal(list[0].allocation.policy, "hard");
+});
+
+test("rowToAllocation preserves valid policies unchanged (Guard A regression)", () => {
+  const pool = poolsDb.createPool({
+    connectionId: "c-guardA-valid",
+    name: "Valid Policy Pool",
+    allocations: [
+      { apiKeyId: "k-hard", weight: 34, policy: "hard" },
+      { apiKeyId: "k-soft", weight: 33, policy: "soft" },
+      { apiKeyId: "k-burst", weight: 33, policy: "burst" },
+    ],
+  });
+
+  const found = poolsDb.getPool(pool.id)!;
+  assert.equal(found.allocations.find((a) => a.apiKeyId === "k-hard")!.policy, "hard");
+  assert.equal(found.allocations.find((a) => a.apiKeyId === "k-soft")!.policy, "soft");
+  assert.equal(found.allocations.find((a) => a.apiKeyId === "k-burst")!.policy, "burst");
+});

tests/unit/quota-combos-sync.test.ts

@@ -332,3 +332,90 @@ test("removeQuotaCombosForPool: unknown pool id — no throw", async () => {
     "removeQuotaCombosForPool with unknown poolId should not throw"
   );
 });
+
+// ---------------------------------------------------------------------------
+// Guard B (issue #10): a pool whose connections no longer resolve (empty/dangling
+// connection list) must NOT prune/delete the group's existing combos. Pruning is
+// provider-scoped, and with no resolvable connection the provider is unknown — so
+// syncQuotaCombos returns early (poolProvider === undefined) BEFORE the prune loop.
+// This proves the `if (!poolProvider) return` guard covers the empty-connection
+// path: a transient connection-resolution failure cannot wipe a group's combos.
+// ---------------------------------------------------------------------------
+
+test("syncQuotaCombos: pool with no resolvable connection does NOT prune existing combos (Guard B)", async () => {
+  // 1. Seed a glm connection + pool and mint combos.
+  const conn = await providersDb.createProviderConnection({
+    provider: "glm",
+    authType: "apikey",
+    name: "quota-combos-guardB",
+    apiKey: "sk-test-glm-guardb",
+  });
+  const connId = (conn as Record<string, unknown>).id as string;
+  const pool = poolsDb.createPool({ connectionId: connId, name: "GuardBPool" });
+
+  await syncQuotaCombos(pool.id);
+  const before = await listQuotaCombos();
+  assert.ok(before.length > 0, "expected combos after initial sync");
+  const beforeNames = new Set(before.map((c) => c.name));
+
+  // 2. Delete the provider connection while the pool still references it.
+  //    The join row (quota_pool_connections) remains → getConnectionIds returns a
+  //    dangling id whose getProviderConnectionById() resolves to null. This is the
+  //    "connections do not resolve" scenario.
+  const removed = await providersDb.deleteProviderConnection(connId);
+  assert.equal(removed, true, "connection should be deleted");
+  assert.equal(
+    await providersDb.getProviderConnectionById(connId),
+    null,
+    "connection must no longer resolve"
+  );
+
+  // 3. Re-sync. With no resolvable connection, poolProvider is undefined → the
+  //    guard returns before pruning. Existing combos must be untouched.
+  await syncQuotaCombos(pool.id);
+
+  const after = await listQuotaCombos();
+  assert.equal(
+    after.length,
+    before.length,
+    "combos must NOT be pruned when the pool has no resolvable connection"
+  );
+  for (const name of beforeNames) {
+    const stillThere = after.some((c) => c.name === name);
+    assert.ok(stillThere, `combo was wrongly pruned: ${name}`);
+  }
+});
+
+test("syncQuotaCombos: pool whose join table is emptied (truly no connectionIds) does NOT prune combos (Guard B)", async () => {
+  // Variant of Guard B where the join table itself is empty AND the primary
+  // connection is gone — connectionIds falls back to [pool.connectionId], which
+  // also fails to resolve. Still must not prune.
+  const conn = await providersDb.createProviderConnection({
+    provider: "glm",
+    authType: "apikey",
+    name: "quota-combos-guardB-empty",
+    apiKey: "sk-test-glm-guardb-empty",
+  });
+  const connId = (conn as Record<string, unknown>).id as string;
+  const pool = poolsDb.createPool({ connectionId: connId, name: "GuardBEmptyPool" });
+
+  await syncQuotaCombos(pool.id);
+  const before = await listQuotaCombos();
+  assert.ok(before.length > 0, "expected combos after initial sync");
+
+  // Empty the join table for this pool AND delete the connection row.
+  const db = core.getDbInstance() as unknown as {
+    prepare: (sql: string) => { run: (...p: unknown[]) => unknown };
+  };
+  db.prepare("DELETE FROM quota_pool_connections WHERE pool_id = ?").run(pool.id);
+  await providersDb.deleteProviderConnection(connId);
+
+  await syncQuotaCombos(pool.id);
+
+  const after = await listQuotaCombos();
+  assert.equal(
+    after.length,
+    before.length,
+    "combos must survive when the pool has an empty/unresolvable connection set"
+  );
+});

tests/unit/quota-fair-share.test.ts

@@ -201,3 +201,49 @@ test("fairShare: empty dimensions → allow:ok", () => {
   assert.equal(result.kind, "allow");
   assert.equal(result.reason, "ok");
 });
+
+// ─── Guard A: unknown/garbage policy must fail SAFE (treated as hard) ─────────
+// Defensive guard for issue #10: a policy value outside hard|soft|burst (e.g. a
+// corrupted DB row read through `row.policy as Policy`) previously fell through
+// every switch case in decideFairShare and returned a silent `allow`
+// (fail-OPEN). It must be treated as the most restrictive policy (hard) so an
+// unknown policy can never bypass fair-share enforcement.
+
+test("fairShare: GUARD-A strict mode, unknown policy over fair_share → block:fair-share (treated as hard)", () => {
+  // globalUsedPercent=0.6 >= 0.5 → strict; consumed=600 > fair_share=500.
+  // policy is garbage ("bogus") — must behave like hard and BLOCK, not allow.
+  const result = decideFairShare({
+    dimensions: [dim({ limit: 1000, consumedTotal: 700, globalUsedPercent: 0.6 })],
+    // Cast through unknown to bypass the Policy type — simulates a corrupted row.
+    allocation: { weight: 50, policy: "bogus" as unknown as "hard" },
+    consumedByThisKey: { "pool1:tokens:hourly": 600 },
+    saturationThreshold: THRESHOLD,
+  });
+  assert.equal(result.kind, "block");
+  assert.equal(result.reason, "fair-share");
+});
+
+test("fairShare: GUARD-A strict mode, unknown policy UNDER fair_share → allow (hard semantics still allow under share)", () => {
+  // Treated as hard: under fair_share is allowed even in strict mode.
+  const result = decideFairShare({
+    dimensions: [dim({ limit: 1000, consumedTotal: 700, globalUsedPercent: 0.7 })],
+    allocation: { weight: 50, policy: "garbage" as unknown as "hard" },
+    consumedByThisKey: { "pool1:tokens:hourly": 300 },
+    saturationThreshold: THRESHOLD,
+  });
+  assert.equal(result.kind, "allow");
+});
+
+test("fairShare: GUARD-A generous mode, unknown policy → hard semantics (no soft penalize / no burst leniency)", () => {
+  // Generous mode (globalUsedPercent=0.3), consumed=600 > fair_share=500, but
+  // consumedTotal=600 < 1000 so hard allows borrowing. Must NOT be flagged as
+  // penalized (that would be soft) and must NOT block (key < global limit).
+  const result = decideFairShare({
+    dimensions: [dim({ limit: 1000, consumedTotal: 600, globalUsedPercent: 0.3 })],
+    allocation: { weight: 50, policy: "weird" as unknown as "hard" },
+    consumedByThisKey: { "pool1:tokens:hourly": 600 },
+    saturationThreshold: THRESHOLD,
+  });
+  assert.equal(result.kind, "allow");
+  assert.equal(result.penalized, undefined, "unknown policy must not get soft penalize semantics");
+});