Reject enterprise model aliases outside current snapshot

[chrarnoldus]

Summary

• require every Enterprise model request to use an exact model ID present in the current models-by-provider snapshot
• reject unregistered aliases and endpoint-less catalog entries such as grok-4.5 and openrouter/free, and filter them from Enterprise organization model selectors
• preserve the managed ~provider/...-latest aliases outside model deny/snapshot enforcement while retaining organization and group provider routing restrictions
• centralize managed latest-alias constants and export them as a shared list
• preserve Teams, custom LLM, direct-BYOK, and Kilo Auto behavior

Verification

• pnpm --filter web test -- --runInBand apps/web/src/lib/ai-gateway/providers/vercel/mapModelIdToVercel.test.ts apps/web/src/lib/model-allow.server.test.ts apps/web/src/lib/organizations/effective-model-access.server.test.ts apps/web/src/lib/ai-gateway/llm-proxy-helpers.test.ts apps/web/src/lib/organizations/legacy-model-restrictions-parity.test.ts apps/web/src/routers/organizations/organization-settings-router.test.ts
• pnpm --filter web typecheck
• pnpm --filter web lint
• pnpm --filter web dependency-cycle-check
• changed-file formatting and git diff --check

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of 267a9aa66 ("correct DeepSeek latest alias"): the new ~deepseek/deepseek-v4-flash-latest managed alias follows the exact pattern of the existing 10 aliases, the isLatestModelAlias switch to exact matching is fail-closed and consistent with every call site (the proxy route already lowercases IDs before policy checks, and downstream alias resolution was always exact-match), and the replacement of the plain deepseek/deepseek-v4-flash-latest hardcoded Vercel mapping with the tilde-alias key is deliberate, test-covered, and aligned with the PR's rejection of unregistered upstream aliases.

Files Reviewed (3 files in incremental diff)

• apps/web/src/lib/ai-gateway/latest-model-aliases.ts - new alias constant + list entry; exact-match isLatestModelAlias verified against all 3 call sites
• apps/web/src/lib/ai-gateway/providers/vercel/mapModelIdToVercel.ts - alias-keyed DeepSeek mapping; pass-through of plain id verified intentional
• apps/web/src/lib/ai-gateway/providers/vercel/mapModelIdToVercel.test.ts - expectations updated consistently with source ordering

Both previous warnings (kilo-auto virtual-tier denial, bot-integration uncaught throw) remain resolved/obsolete per prior commits; no unresolved findings.

Previous Review Summaries (7 snapshots, latest commit 2a6a549)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 2a6a549)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of 2a6a5499e (revert bot-integration policy changes to descope PR): the Slack/Discord/Linear upsert and shared bot-default changes were reverted byte-identical to base — all getDefaultAllowedModel call sites again match the restored Promise<string> signature, TRPCError imports stay in use, and both previous warnings (kilo-auto virtual tiers, uncaught throw) are resolved or obsolete.

Files Reviewed (6 files, all reverted out of the PR diff)

• apps/web/src/lib/integrations/discord-service.test.ts - null-policy test and mock removed; back to base state
• apps/web/src/lib/integrations/discord-service.ts - null-check throw removed; identical to base
• apps/web/src/lib/integrations/linear-service.ts - policy lookup reordered and null-check removed; identical to base
• apps/web/src/lib/integrations/slack-service.ts - existing-slug preservation inlined and null-check removed; identical to base
• apps/web/src/lib/slack-bot/model-allow-list.test.ts - deleted; file never existed on base
• apps/web/src/lib/slack-bot/model-allow-list.ts - Promise<string> signature and globalDefault fallback restored; identical to base

Remaining PR scope (snapshot enforcement in model-allow.server.ts, model-access.server.ts, model-restrictions.ts, routers, and tests) is unchanged since the previous review at 9957bea33, which found no issues.

Previous review (commit 9957bea)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of 9957bea33 (handle model policy edge cases): the previous uncaught-throw warning is fully resolved — getDefaultAllowedModel now returns null, all four call sites handle it (Slack/Discord/Linear upserts raise an actionable BAD_REQUEST, Discord message handling accepts null), alias snapshot lookups are consistently removed from both the legacy predicate and the group-policy evaluator, and the removed resolveLatestModelAlias/GROK_CURRENT_MODEL_ID exports have no dangling references.

Files Reviewed (13 files)

• apps/web/src/lib/ai-gateway/latest-model-aliases.ts - alias→target map removal is clean; no remaining references repo-wide
• apps/web/src/lib/ai-gateway/providers/vercel/mapModelIdToVercel.test.ts - removed test/imports leave no unused imports
• apps/web/src/lib/ai-gateway/providers/xai.ts - GROK_CURRENT_MODEL_ID removal has no remaining usages
• apps/web/src/lib/integrations/discord-service.test.ts - new null-policy test traces correctly against the upsert guard
• apps/web/src/lib/integrations/discord-service.ts - null check throws actionable TRPCError before insert; existing installs return early; getModel null propagates to a handled path in discord-bot.ts
• apps/web/src/lib/integrations/linear-service.ts - existing model_slug preserved; policy lookup only when absent; null guard correct
• apps/web/src/lib/integrations/slack-service.ts - same preserved-slug + null-guard pattern; TRPCError import present
• apps/web/src/lib/model-allow.server.test.ts - updated alias expectations match new predicate semantics (non-empty allow list passes, empty denies)
• apps/web/src/lib/model-allow.server.ts - alias branch defers provider compatibility to inference routing per PR intent; empty-allow-list denial consistent with non-alias path
• apps/web/src/lib/organizations/effective-model-access.server.test.ts - throwing lookup correctly asserts aliases never hit snapshot provider metadata
• apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts - alias path skips deny/snapshot lookups and intersects grant/ceiling routes coherently
• apps/web/src/lib/slack-bot/model-allow-list.test.ts - null-result test coherent with restricted-policy mocks
• apps/web/src/lib/slack-bot/model-allow-list.ts - string | null return resolves the previous warning; all call sites updated

Previous review (commit 701658e)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

Incremental review of 701658ea5 (keep snapshot policies consistent): alias→target snapshot provider resolution is coherent across the legacy predicate and the group-policy evaluator, but the new fail-closed throw in getDefaultAllowedModel propagates uncaught through Slack/Discord/Linear integration call sites.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/slack-bot/model-allow-list.ts	65	New throw for fully-restricted orgs propagates uncaught through bot OAuth upsert and getModel call sites (previously fell back to globalDefault); defaults route analog returns a structured 409

Files Reviewed (12 files)

• apps/web/src/app/api/organizations/[id]/defaults/route.test.ts - new enterprise 409 test traces correctly against the route
• apps/web/src/app/api/organizations/[id]/defaults/route.ts - unrestricted shortcut now honors requireModelInCurrentSnapshot; matches PR intent
• apps/web/src/lib/ai-gateway/latest-model-aliases.ts - alias→target map complete for all 10 aliases; bounded module-level Map, no leak
• apps/web/src/lib/ai-gateway/providers/vercel/mapModelIdToVercel.test.ts - resolveLatestModelAlias coverage added
• apps/web/src/lib/ai-gateway/providers/xai.ts - GROK_CURRENT_MODEL_ID uses OpenRouter-style id, distinct from Vercel id as required by snapshot lookups
• apps/web/src/lib/model-allow.server.test.ts - compatible/incompatible/empty provider-list alias cases all trace correctly
• apps/web/src/lib/model-allow.server.ts - alias branch checks target snapshot providers against the allow list with no-route fallback; deny/snapshot bypass retained by design
• apps/web/src/lib/organizations/effective-model-access.server.test.ts - lookup now expects the resolved target id; expectations match evaluator logic
• apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts - alias ceiling pass-through narrowed to empty-snapshot targets; grant substitution and intersections coherent
• apps/web/src/lib/slack-bot/model-allow-list.test.ts - snapshot fallback test coherent with mocks
• apps/web/src/lib/slack-bot/model-allow-list.ts - 1 issue: uncaught throw at call sites
• apps/web/src/routers/organizations/organization-modes-router.test.ts - provider-index mock scoped correctly; getModelIdToProviderSlugsIndex unreachable from this router graph

Previous review (commit 683720d)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 683720d3d (preserve managed ~provider/...-latest aliases outside model deny/snapshot enforcement): the new shared latest-model-aliases module is consumed consistently across the LLM proxy deny-list check, the legacy allow predicate, and the group-policy evaluator; alias bypass is correctly ordered after the custom-LLM/BYOK/kilo-auto exemptions, snapshot provider lookups are skipped only for aliases, and organization/group provider routing restrictions are retained via the provider ceiling and grant intersections, with all new tests tracing correctly against the evaluator logic.

Files Reviewed (10 files)

• apps/web/src/lib/ai-gateway/latest-model-aliases.ts - new shared constants and case-insensitive isLatestModelAlias lookup; module-level Set, no leak
• apps/web/src/lib/ai-gateway/llm-proxy-helpers.test.ts - alias bypass case retains provider config only
• apps/web/src/lib/ai-gateway/llm-proxy-helpers.ts - deny-list guard skips aliases while provider allow list still applies
• apps/web/src/lib/ai-gateway/providers/vercel/mapModelIdToVercel.test.ts - alias cases switched to shared constants plus list-completeness test
• apps/web/src/lib/ai-gateway/providers/vercel/mapModelIdToVercel.ts - mapping keys switched to shared constants; mapping unchanged
• apps/web/src/lib/model-allow.server.test.ts - alias allowed with providers, denied with empty provider allow list
• apps/web/src/lib/model-allow.server.ts - alias branch placed after exemptions and before deny/snapshot checks; provider compatibility deferred to inference routing
• apps/web/src/lib/organizations/effective-model-access.server.test.ts - alias bypass cases verified, including providerLookup-throws guard and empty-provider denial
• apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts - alias skips deny list and snapshot lookup; ceiling pass-through and member-grant substitution are coherent
• apps/web/src/routers/organizations/organization-settings-router.test.ts - alias remains in available models despite deny list and missing snapshot routes

Previous review (commit 05a60f0)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 05a60f007 (constrain all enterprise models to snapshot; the intervening main merge did not touch PR files): the new requireModelInCurrentSnapshot flag is threaded consistently through the legacy predicate, group-policy evaluator, auto-target validation, and settings router, defaults preserve all existing callers, exemptions (custom-LLM, direct-BYOK, kilo-auto) still precede deny/snapshot checks, and all new/updated tests trace correctly against the mocked provider lookups. The previous kilo-auto warning remains resolved.

Files Reviewed (9 files)

• apps/web/src/lib/model-allow.server.ts - new optional requireModelInCurrentSnapshot param defaults to false; early-return guard and exemption ordering verified
• apps/web/src/lib/model-allow.server.test.ts - new snapshot-membership cases consistent with predicate semantics
• apps/web/src/lib/organizations/effective-model-access.server.test.ts - enterprise-without-restrictions snapshot denial case verified against evaluator logic
• apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts - flag is true exactly when plan is enterprise; lookup reuse and denial ordering intact
• apps/web/src/lib/organizations/legacy-model-restrictions-parity.test.ts - new parity case matches manual trace of both legacy predicate and evaluator
• apps/web/src/lib/organizations/model-restrictions.ts - plan-derived flag matches evaluator derivation
• apps/web/src/lib/organizations/organization-auto-model.ts - flag derived from plan; kilo-auto tier and catalog checks unaffected
• apps/web/src/routers/organizations/organization-settings-router.test.ts - updated expectations align with mocked snapshot lookups
• apps/web/src/routers/organizations/organization-settings-router.ts - hardcoded requireModelInCurrentSnapshot: true in updateAllowLists is safe behind the enterprise plan gate

Previous review (commit af6afb9)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commits aa076fadc and af6afb91f: the previous kilo-auto warning is resolved (virtual tiers now share the custom-LLM/direct-BYOK exemption with test coverage in all three enforcement paths), and the requireModelInCurrentSnapshot flag removal derives snapshot enforcement identically at every former computation site.

Files Reviewed (8 files)

• apps/web/src/lib/ai-gateway/model-utils.ts - new KILO_AUTO_MODEL_PREFIX constant and doc update verified; no duplicate declaration conflicts
• apps/web/src/lib/model-allow.server.ts - derived snapshot enforcement matches removed flag semantics; exemption-before-deny ordering preserved; early return reorder is outcome-identical; provider_allow_list is never null per zod schema so truthy/undefined checks cannot diverge
• apps/web/src/lib/model-allow.server.test.ts - updated deny-only test correctly codifies intentional snapshot requirement; kilo-auto exemption case added
• apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts - hasOrganizationRestrictions derivation is the identical expression previously stored in the removed flag; kilo-auto exemption resolves the previous review warning
• apps/web/src/lib/organizations/effective-model-access.server.test.ts - exemption coverage added for evaluator path
• apps/web/src/lib/organizations/model-restrictions.ts - flag removal safe; predicate derives the same condition from identical inputs
• apps/web/src/routers/organizations/organization-settings-router.ts - updateAllowLists default-model revalidation derives the same condition from the same merged-settings inputs; audit flow intact
• apps/web/src/routers/organizations/organization-settings-router.test.ts - kilo-auto default exemption coverage added

Previous review (commit 0877964)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The snapshot-membership requirement is correctly implemented for alias rejection, but it also silently denies Kilo's own kilo-auto/* virtual tiers for restricted Enterprise orgs across the model-list and defaults endpoints, which is untested and inconsistent with the gateway request path.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts	125	kilo-auto/* virtual tiers are absent from the models-by-provider snapshot, so they now evaluate as denied for any Enterprise org with an active deny list or provider ceiling — they disappear from the org model list, defaultFreeModel becomes null, and kilo-auto defaults are rejected/cleared — while the gateway still permits auto-routed requests after per-candidate policy checks. Confirm intent (add a test) or exempt kilo-auto/ in isModelRestrictionExempt.

Files Reviewed (8 files)

• apps/web/src/lib/model-allow.server.ts - predicate logic verified: exemption precedes deny list (intentional, matches custom-LLM/direct-BYOK AGENTS.md invariant), unrestricted/Teams paths preserved, no regression for non-snapshot callers
• apps/web/src/lib/model-allow.server.test.ts - no issues
• apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts - 1 issue (kilo-auto side effect); lookup reuse via currentModelProviders ?? verified, denial-source change consistent
• apps/web/src/lib/organizations/effective-model-access.server.test.ts - no issues
• apps/web/src/lib/organizations/legacy-model-restrictions-parity.test.ts - parity expectations updated consistently on both sides
• apps/web/src/lib/organizations/model-restrictions.ts - flag derivation matches evaluator; Teams still returns unrestricted
• apps/web/src/routers/organizations/organization-settings-router.ts - merged-settings flag computation correct; audit flow intact
• apps/web/src/routers/organizations/organization-settings-router.test.ts - new default mock covers all unmodified tests that reach the predicate

Fix these issues in Kilo Cloud

---

_{Reviewed by kimi-k3 · Input: 108.1K · Output: 15.1K · Cached: 757.7K}

_{Review guidance: REVIEW.md from base branch main}