[MumuTW]

Summary

Model authorization currently lives inside API-key persistence, mixing the allow/block, publication, and group decision flow with metadata loading and caching. Extract that decision flow into src/domain/modelAccessPolicy.ts, with pure matching helpers in src/domain/modelPermissions.ts.

The existing isModelAllowedForKey(): Promise<boolean> facade supplies lazy lookup ports and keeps metadata, runtime settings, and permission caching in the DB layer. Existing helper imports continue to work through compatibility exports.

Preserved behavior

• Deny patterns take priority; requested aliases and resolved targets retain their existing matching and group checks.
• Restricted mode with an empty allow-list stays deny-all. Groups can only narrow a per-key grant.
• Publication checks retain provider aliases, hidden models, declared effort variants, and the Claude wildcard exception.
• Lookup order and short-circuiting are unchanged. Early block/publication denials remain uncached, and setting-dependent Claude routing still bypasses permission cache reuse.

flowchart TD
  Callers["Chat, catalog, and combo callers"] --> Wrapper["isModelAllowedForKey<br/>DB facade"]
  Wrapper --> Metadata["Key metadata and permission cache"]
  Wrapper -->|"policy, target, lazy lookups"| Policy["evaluateModelAccess<br/>Domain policy"]
  Policy --> Matching["Pure model matching"]
  Policy -->|"only when needed"| Adapters["Candidate, publication, and group adapters"]
  Adapters --> Storage["Existing settings, catalog, and group storage"]
  Policy -->|"decision and reason"| Wrapper

Loading

Validation

• Characterization before wiring: 112 existing tests and 6 new wrapper tests passed against the original implementation.
• After extraction: 160/160 focused tests passed, including the 22 new domain/wrapper cases and existing catalog, group, resolved-target, combo-target, and exclusive-lease guards.
• Focused TypeScript check for all six changed TypeScript files, with imported dependencies and strictNullChecks: true: passed.
• npm run lint: passed.
• npm run check:migration-numbering: passed.
• npm run check:db-rules: passed.
• CHANGELOG_BASE_REF=upstream/release/v3.8.53 npm run check:changelog-integrity: passed.
• Normal pre-commit hooks: passed. Full unit/Vitest shards, coverage ratchet, and build remain CI validation.

Focused test command:

DISABLE_SQLITE_AUTO_BACKUP=true node \
  --import tsx/esm \
  --import ./open-sse/utils/setupPolyfill.ts \
  --import ./tests/_setup/isolateDataDir.ts \
  --test --test-force-exit --test-concurrency=2 \
  tests/unit/db-apiKeys-crud.test.ts \
  tests/unit/api-manager-provider-permissions.test.ts \
  tests/unit/model-catalog-policy-invalidation-8728.test.ts \
  tests/unit/api-key-resolved-model-cache-12899.test.ts \
  tests/unit/api-key-group-inactive-fail-closed.test.ts \
  tests/unit/apikey-disable-non-public-provider-alias.test.ts \
  tests/unit/apikey-disable-non-public-effort-variant.test.ts \
  tests/unit/api-key-catalog-claude-bare-root-leak.test.ts \
  tests/unit/apikeys-model-permissions-split.test.ts \
  tests/unit/model-access-policy.test.ts \
  tests/unit/model-access-policy-wrapper.test.ts \
  tests/unit/resolved-model-access.test.ts \
  tests/unit/combo-restricted-key-target-policy-12886.test.ts \
  tests/unit/exclusive-lease-api-key-policy.test.ts

Tests added and coverage

• tests/unit/model-access-policy.test.ts: 16 cases covering decision precedence, lazy short-circuits, publication rules, group narrowing, and domain import/evaluation without SQLite initialization.
• tests/unit/model-access-policy-wrapper.test.ts: 6 DB-backed characterization cases covering requested/resolved identities, group restrictions, and resolved-target cache isolation.
• Existing matching, alias, visibility, invalidation, and setting-dependent cache tests continue to exercise the moved code. Tests use isolated temporary data directories.

Reviewer notes

• Change type: DB/domain refactor; no migration or feature flag.
• Base: release/v3.8.53 at daca9902e859bc1aa95ba276e3ef200ba7f5d142.
• Independent of fix(api): preserve search-provider chat rejection responses #17953 and refactor(routing): share combo attempt failure facts #17956; neither PR's commits are included.
• ⚠️ base-red inherited: 🔴 Release branch not green: release/v3.8.53 #17219. Existing release failures are outside this PR.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-22T19:28:46.828299Z	cb55792	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb55792432

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[diegosouzapw]

Thanks @MumuTW, this is a very clean extraction and an unusually well-documented one. Pulling the allow/block/publication/group decision out of key storage into a domain function with lazy lookup ports makes the most security-sensitive code path in the key layer much easier to reason about.

What we checked: we ran a differential fuzz of the original inline logic against evaluateModelAccess over 60k random policies (allow/block patterns including cc/, claude/, provider/*, wildcards, restricted plus an empty list, disableNonPublicModels, group ids, resolved targets and random lookup results). It gave identical decisions, identical lookup call order and the same cache/no-cache boundary every time, and the moved matchers are byte-identical to the originals. All the focused suites you listed pass on our side, and typecheck:core is clean. The commit metadata is also clean, so the automated "Codex author" comment looks stale for this head.

One small suggestion, not a blocker: the "early block/publication denials stay uncached" invariant in your summary is not pinned by any test. When we flipped the wrapper to cache those denials, all the relevant tests still passed. A short wrapper-level test (deny by blockedModels, then by an unpublished model, and assert nothing is written to the permission cache) would lock it in. If you would rather not, we can add it ourselves in-place.

The base branch is currently red from the inherited #17219 failures, so CI noise on unit shards or the README docs-sync check is not from this PR.