Merge pull request #300 from mcowger/claude/issue-299-20260426-1749

feat: add excludedModels/excludedProviders to key access policy

Author: mcowger

AGENTS.md

@@ -82,15 +82,14 @@ Backend tests run on **Vitest** with two parallel projects (sqlite + postgres) d
 | Setting | Value | Effect |
 |---------|-------|--------|
 | `pool` | `forks` | Each test file runs in a child process fork |
-| `isolate` | `false` | All files in one worker share the **same module registry** — no reset between files |
-| `maxWorkers` | `1` | One fork at a time (sequential files within each project) |
+| `isolate` | `true` | Each test file gets its own module registry — modules are re-imported fresh per file |
 | `mockReset` | `true` | `vi.resetAllMocks()` before every test — clears `vi.fn()` call history and resets implementations to original |
 | `clearMocks` | `true` | Clears call history (redundant with mockReset but explicit) |
 | `restoreMocks` | `true` | Restores `vi.spyOn` mocks to originals after every test |
 
 **Global setup:** `packages/backend/test/vitest.global-setup.ts` — creates a temporary DB, runs migrations once, cleans up after the run. Runs once total.
 
-**Per-file setup:** `packages/backend/test/vitest.setup.ts` — **runs once per test file** (even with `isolate: false`). Installs the logger mock and the `@mariozechner/pi-ai` mock.
+**Per-file setup:** `packages/backend/test/vitest.setup.ts` — **runs once per test file**. Installs the logger mock and the `@mariozechner/pi-ai` mock.
 
 **Rule 4 — Test shared mutable state through the system under test, not through direct mutation.**
 The `utils/logger` mock in `vitest.setup.ts` closes over a `currentLogLevel` variable that both the mock functions and route handlers share. Adding a competing `vi.mock` for the same path in a test file can bind different closures to different variables, causing silent mismatches. Tests for stateful behaviour (e.g., logging routes) must interact exclusively through the API/HTTP layer — never by calling `setCurrentLogLevel` or `getCurrentLogLevel` directly from test code.
@@ -135,14 +134,14 @@ This means the global mock's `complete: vi.fn(async () => ({...}))` is safe —
 
 ### Singletons and test isolation
 
-Several services are singletons (e.g., `OAuthAuthManager`, `CooldownManager`, `DebugManager`). Always reset them in `beforeEach`/`afterEach` using their provided `resetForTesting()` or equivalent methods. With `isolate: false`, a singleton left in a non-default state in one test leaks into every subsequent test in the same worker.
+Several services are singletons (e.g., `OAuthAuthManager`, `CooldownManager`, `DebugManager`). Always reset them in `beforeEach`/`afterEach` using their provided `resetForTesting()` or equivalent methods. With `pool: forks` and `isolate: true`, singletons are re-initialized per file, but can still leak state between tests within the same file.
 
 ### Other rules
 - `packages/backend/bunfig.toml` blocks raw `bun test` — use `bun run test` / `bun run test:watch`
 - Root `bunfig.toml` blocks raw `bun test` at repo root — use `cd packages/backend && bun run test`
 - **Prefer `bun run test` (affected only) over `bun run test:force-all`.** The default test command uses `--changed HEAD` and runs only tests affected by uncommitted changes — use it unless you have a specific reason to run the full suite (e.g., verifying a cross-cutting refactor or diagnosing flakiness unrelated to your changes). Never reach for `test:force-all` out of habit.
 - If you must mock a module, implement its **full public interface**
-- Do not use `__mocks__` directories for node_modules mocks — they are not reliably loaded with `isolate: false` when the real module may already be cached
+- Do not use `__mocks__` directories for node_modules mocks — they are not reliably loaded by Vitest with `pool: forks`
 
 ---
 

packages/backend/drizzle/schema/postgres/api-keys.ts

@@ -9,6 +9,8 @@ export const apiKeys = pgTable('api_keys', {
   quotaName: text('quota_name'),
   allowedModels: text('allowed_models'),
   allowedProviders: text('allowed_providers'),
+  excludedModels: text('excluded_models'),
+  excludedProviders: text('excluded_providers'),
   createdAt: bigint('created_at', { mode: 'number' }).notNull(),
   updatedAt: bigint('updated_at', { mode: 'number' }).notNull(),
 });

packages/backend/drizzle/schema/sqlite/api-keys.ts

@@ -9,6 +9,8 @@ export const apiKeys = sqliteTable('api_keys', {
   quotaName: text('quota_name'),
   allowedModels: text('allowed_models'),
   allowedProviders: text('allowed_providers'),
+  excludedModels: text('excluded_models'),
+  excludedProviders: text('excluded_providers'),
   createdAt: integer('created_at').notNull(),
   updatedAt: integer('updated_at').notNull(),
 });

packages/backend/src/config.ts

@@ -555,6 +555,8 @@ export const KeyConfigSchema = z.object({
   quota: z.string().optional(), // References a quota definition name
   allowedModels: z.array(z.string().min(1)).optional(),
   allowedProviders: z.array(z.string().min(1)).optional(),
+  excludedModels: z.array(z.string().min(1)).optional(),
+  excludedProviders: z.array(z.string().min(1)).optional(),
 });
 
 const QuotaConfigSchema = z.object({

packages/backend/src/db/config-repository.ts

@@ -753,13 +753,17 @@ export class ConfigRepository {
     for (const row of rows) {
       const allowedModels = parseStringArray(row.allowedModels);
       const allowedProviders = parseStringArray(row.allowedProviders);
+      const excludedModels = parseStringArray(row.excludedModels);
+      const excludedProviders = parseStringArray(row.excludedProviders);
 
       result[row.name] = {
         secret: decrypt(row.secret),
         ...(row.comment ? { comment: row.comment } : {}),
         ...(row.quotaName ? { quota: row.quotaName } : {}),
         ...(allowedModels ? { allowedModels } : {}),
         ...(allowedProviders ? { allowedProviders } : {}),
+        ...(excludedModels ? { excludedModels } : {}),
+        ...(excludedProviders ? { excludedProviders } : {}),
       };
     }
 
@@ -798,6 +802,8 @@ export class ConfigRepository {
     const row = rows[0]!;
     const allowedModels = parseStringArray(row.allowedModels);
     const allowedProviders = parseStringArray(row.allowedProviders);
+    const excludedModels = parseStringArray(row.excludedModels);
+    const excludedProviders = parseStringArray(row.excludedProviders);
 
     return {
       name: row.name,
@@ -807,6 +813,8 @@ export class ConfigRepository {
         ...(row.quotaName ? { quota: row.quotaName } : {}),
         ...(allowedModels ? { allowedModels } : {}),
         ...(allowedProviders ? { allowedProviders } : {}),
+        ...(excludedModels ? { excludedModels } : {}),
+        ...(excludedProviders ? { excludedProviders } : {}),
       },
     };
   }
@@ -833,6 +841,8 @@ export class ConfigRepository {
           quotaName: config.quota ?? null,
           allowedModels: stringifyStringArray(config.allowedModels),
           allowedProviders: stringifyStringArray(config.allowedProviders),
+          excludedModels: stringifyStringArray(config.excludedModels),
+          excludedProviders: stringifyStringArray(config.excludedProviders),
           updatedAt: timestamp,
         })
         .where(eq(schema.apiKeys.name, name));
@@ -847,6 +857,8 @@ export class ConfigRepository {
           quotaName: config.quota ?? null,
           allowedModels: stringifyStringArray(config.allowedModels),
           allowedProviders: stringifyStringArray(config.allowedProviders),
+          excludedModels: stringifyStringArray(config.excludedModels),
+          excludedProviders: stringifyStringArray(config.excludedProviders),
           createdAt: timestamp,
           updatedAt: timestamp,
         });

packages/backend/src/routes/inference/__tests__/auth.test.ts

@@ -522,3 +522,85 @@ describe('Key Access Policy Propagation', () => {
     });
   });
 });
+
+describe('Key Access Policy Exclusion Propagation', () => {
+  let fastify: FastifyInstance;
+  let mockUsageStorage: UsageStorageService;
+  let capturedRequest: any;
+
+  beforeAll(async () => {
+    fastify = Fastify();
+    capturedRequest = null;
+
+    const mockDispatcher = {
+      dispatch: vi.fn(async (request: any) => {
+        capturedRequest = request;
+        return {
+          id: '123',
+          model: 'gpt-4',
+          created: 123,
+          content: 'test content',
+          usage: { input_tokens: 10, output_tokens: 10, total_tokens: 20 },
+        };
+      }),
+    } as unknown as Dispatcher;
+
+    mockUsageStorage = {
+      saveRequest: vi.fn(),
+      saveError: vi.fn(),
+      updatePerformanceMetrics: vi.fn(),
+      emitStartedAsync: vi.fn(),
+      emitUpdatedAsync: vi.fn(),
+    } as unknown as UsageStorageService;
+
+    DebugManager.getInstance().setStorage(mockUsageStorage);
+    SelectorFactory.setUsageStorage(mockUsageStorage);
+
+    setConfigForTesting({
+      providers: {},
+      models: {
+        'gpt-4': {
+          priority: 'selector',
+          targets: [{ provider: 'openai', model: 'gpt-4' }],
+        },
+      },
+      keys: {
+        excluded: {
+          secret: 'sk-excluded-key',
+          excludedModels: ['claude-3-opus', 'gpt-5'],
+          excludedProviders: ['azure-openai', 'google'],
+        },
+      },
+      failover: {
+        enabled: false,
+        retryableStatusCodes: [429, 500, 502, 503, 504],
+        retryableErrors: ['ECONNREFUSED', 'ETIMEDOUT'],
+      },
+      quotas: [],
+    });
+
+    await registerInferenceRoutes(fastify, mockDispatcher, mockUsageStorage);
+    await fastify.ready();
+  });
+
+  it('attaches excluded models and providers policy metadata', async () => {
+    const response = await fastify.inject({
+      method: 'POST',
+      url: '/v1/chat/completions',
+      headers: {
+        authorization: 'Bearer sk-excluded-key',
+        'content-type': 'application/json',
+      },
+      payload: {
+        model: 'gpt-4',
+        messages: [],
+      },
+    });
+
+    expect(response.statusCode).toBe(200);
+    expect(capturedRequest?.metadata?.plexus_metadata?.plexus_key_policy).toEqual({
+      excludedModels: ['claude-3-opus', 'gpt-5'],
+      excludedProviders: ['azure-openai', 'google'],
+    });
+  });
+});

packages/backend/src/routes/management.ts

@@ -60,6 +60,8 @@ export async function registerManagementRoutes(
         keyName: p.keyName,
         allowedProviders: p.allowedProviders,
         allowedModels: p.allowedModels,
+        excludedProviders: p.excludedProviders,
+        excludedModels: p.excludedModels,
         quotaName: p.quotaName ?? null,
         comment: p.comment ?? null,
       });

packages/backend/src/routes/management/_principal.ts

@@ -31,6 +31,8 @@ export type Principal =
       keyName: string;
       allowedProviders: string[];
       allowedModels: string[];
+      excludedProviders: string[];
+      excludedModels: string[];
       quotaName?: string | null;
       comment?: string | null;
     };
@@ -94,6 +96,8 @@ export async function resolvePrincipal(request: FastifyRequest): Promise<Princip
     const cfg = matched.cfg as {
       allowedProviders?: string[];
       allowedModels?: string[];
+      excludedProviders?: string[];
+      excludedModels?: string[];
       quota?: string | null;
       comment?: string | null;
     };
@@ -102,6 +106,8 @@ export async function resolvePrincipal(request: FastifyRequest): Promise<Princip
       keyName: matched.name,
       allowedProviders: cfg.allowedProviders ?? [],
       allowedModels: cfg.allowedModels ?? [],
+      excludedProviders: cfg.excludedProviders ?? [],
+      excludedModels: cfg.excludedModels ?? [],
       quotaName: cfg.quota ?? null,
       comment: cfg.comment ?? null,
     };

packages/backend/src/routes/management/self.ts

@@ -67,6 +67,8 @@ export async function registerSelfRoutes(fastify: FastifyInstance, quotaEnforcer
       keyName: principal.keyName,
       allowedProviders: principal.allowedProviders,
       allowedModels: principal.allowedModels,
+      excludedProviders: principal.excludedProviders,
+      excludedModels: principal.excludedModels,
       quotaName: principal.quotaName ?? null,
       comment: keyRow?.comment ?? principal.comment ?? null,
       traceEnabled: DebugManager.getInstance().isEnabledForKey(principal.keyName),

packages/backend/src/services/__tests__/dispatcher-failover.test.ts

@@ -460,4 +460,104 @@ describe('Dispatcher Failover', () => {
 
     expect(fetchMock).toHaveBeenCalledTimes(0);
   });
+
+  test('provider denylist filters out excluded providers', async () => {
+    setConfigForTesting(makeConfig({ targetCount: 2 }));
+    fetchMock.mockImplementation(async () => successChatResponse('model-1'));
+
+    const dispatcher = new Dispatcher();
+    const response = await dispatcher.dispatch({
+      ...makeChatRequest(),
+      metadata: {
+        plexus_metadata: {
+          plexus_key_policy: {
+            excludedProviders: ['p1'],
+          },
+        },
+      },
+    });
+    const meta = (response as any).plexus;
+
+    expect(meta?.attemptCount).toBe(1);
+    expect(meta?.finalAttemptProvider).toBe('p2');
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(String((fetchMock as any).mock.calls[0]?.[0])).toContain('p2.example.com');
+  });
+
+  test('model denylist blocks excluded aliases with 403 before fetch', async () => {
+    setConfigForTesting(makeConfig({ targetCount: 2 }));
+
+    const dispatcher = new Dispatcher();
+
+    await expect(
+      dispatcher.dispatch({
+        ...makeChatRequest(),
+        metadata: {
+          plexus_metadata: {
+            plexus_key_policy: {
+              excludedModels: ['test-alias'],
+            },
+          },
+        },
+      })
+    ).rejects.toMatchObject({
+      message: "Key is not allowed to access model 'test-alias' for chat",
+      routingContext: {
+        statusCode: 403,
+        errorType: 'access_denied',
+      },
+    });
+
+    expect(fetchMock).toHaveBeenCalledTimes(0);
+  });
+
+  test('excluded models take precedence over allowed models for the same entry', async () => {
+    setConfigForTesting(makeConfig({ targetCount: 2 }));
+
+    const dispatcher = new Dispatcher();
+
+    // If a model is in both allowed AND excluded lists, excluded wins
+    await expect(
+      dispatcher.dispatch({
+        ...makeChatRequest(),
+        metadata: {
+          plexus_metadata: {
+            plexus_key_policy: {
+              allowedModels: ['test-alias'],
+              excludedModels: ['test-alias'],
+            },
+          },
+        },
+      })
+    ).rejects.toMatchObject({
+      routingContext: {
+        statusCode: 403,
+        errorType: 'access_denied',
+      },
+    });
+
+    expect(fetchMock).toHaveBeenCalledTimes(0);
+  });
+
+  test('excluded providers filter out candidates, then allowed providers further restrict', async () => {
+    setConfigForTesting(makeConfig({ targetCount: 2 }));
+    fetchMock.mockImplementation(async () => successChatResponse('model-2'));
+
+    const dispatcher = new Dispatcher();
+    const response = await dispatcher.dispatch({
+      ...makeChatRequest(),
+      metadata: {
+        plexus_metadata: {
+          plexus_key_policy: {
+            excludedProviders: ['p1'],
+            allowedProviders: ['p2'],
+          },
+        },
+      },
+    });
+    const meta = (response as any).plexus;
+
+    expect(meta?.attemptCount).toBe(1);
+    expect(meta?.finalAttemptProvider).toBe('p2');
+  });
 });