feat(models): make per-agent model allowlist explicit; aliases/defaults no longer restrict#110888
Conversation
openclaw-barnacle
Bot
added
docs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8371f52242
ℹ️ 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".
| if (params.agentId) { | ||
| const normalizedAgentId = normalizeAgentId(params.agentId); | ||
| const agent = params.cfg?.agents?.list?.find( | ||
| (entry) => normalizeAgentId(entry.id) === normalizedAgentId, | ||
| ); | ||
| if (hasExplicitModelPolicyAllow(agent?.modelPolicy)) { |
There was a problem hiding this comment.
When an agent’s modelPolicy.allow differs from the default policy, Gateway sessions.patch validates against the wrong scope: src/gateway/sessions-patch.ts:625-631 calls resolveAllowedModelRef without the already-resolved sessionAgentId, so this new per-agent branch is never entered. Consequently, the Control UI/session API rejects models permitted only for that agent, while models denied by that agent but permitted globally are accepted and persisted before runtime later reverts them; per-agent aliases are likewise unavailable on this path. Propagate agentId through resolveAllowedModelRef, its alias index, and its status check.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 4:06 PM ET / 20:06 UTC. Summary Reproducibility: no. The described regression has a plausible gateway/config path and targeted tests, but no inspectable final-head live transcript or artifact establishes the before/after result. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land only after a maintainer accepts the explicit persistent policy contract and inspectable, redacted gateway evidence proves unrestricted alias/default changes, explicit rejection, and legacy migration behavior on the final head. Do we have a high-confidence way to reproduce the issue? No. The described regression has a plausible gateway/config path and targeted tests, but no inspectable final-head live transcript or artifact establishes the before/after result. Is this the best way to solve the issue? Unclear. Separating aliases and defaults from authorization is a coherent solution, but the permanent public config and upgrade semantics need maintainer approval and final-head proof before it can be called the best fix. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 1cacb12c4d7b. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
3557022 to
5a7a1d3
Compare
|
/allow-security-sensitive-change 5a7a1d3 |
|
/allow-dependencies-change 5a7a1d3 |
clawsweeper
Bot
removed
the
rating: 🦪 silver shellfish
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a7a1d32f7
ℹ️ 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".
| } | ||
|
|
||
| /** Build allowed model keys/catalog entries after provider wildcards and fallbacks. */ | ||
| /** Build explicit override authorization plus configured automatic fallback keys. */ |
There was a problem hiding this comment.
When the configured primary is outside a wildcard policy—for example, primary anthropic/claude-sonnet-4-6 with modelPolicy.allow: ["openai/*"]—this function omits the primary from allowedKeys and allowedCatalog. Both src/agents/command/model-selection.ts:341 and src/auto-reply/reply/model-selection.ts:437 then call resolveSelection for an ordinary run with no override, so OpenClaw silently substitutes the first allowed OpenAI model or aborts when no catalog row is available, even though the new contract defines this as an override-only allowlist. Preserve the configured primary for initial selection independently of explicit override authorization.
Useful? React with 👍 / 👎.
| const stampedOutputConfig = stampConfigVersion( | ||
| outputConfig, | ||
| options.lastTouchedVersionOverride, | ||
| snapshot.exists ? snapshot.parsed : null, | ||
| ); |
There was a problem hiding this comment.
When a legacy agents.defaults.models restriction is owned by a nested $include, snapshot.parsed contains only the include directive, not the resolved model map. The write therefore finds no legacy refs but still stamps meta.migrations.modelPolicyAllowlist; on the next read, that marker makes computeModelPolicyAllowlist ignore the included legacy map, silently changing the installation to allow-any after any unrelated config write. Detect the legacy restriction from the resolved/source snapshot while preserving the authored include structure.
Useful? React with 👍 / 👎.
5a7a1d3 to
afc6e60
Compare
|
/allow-security-sensitive-change afc6e60 |
|
/allow-dependencies-change afc6e60 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afc6e60f67
ℹ️ 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".
| allowAny: true, | ||
| allowedCatalog: catalog, | ||
| allowedKeys: catalogKeys, |
There was a problem hiding this comment.
When a schema-valid policy contains only blank refs, such as modelPolicy.allow: [" "], parseConfiguredModelVisibilityEntries treats the array as restrictive but every ref is skipped; this branch then converts the policy to allowAny. Callers that omit defaultModel, notably the x-openclaw-model check in src/gateway/http-utils.ts, consequently permit arbitrary model overrides despite a non-empty policy. Reject blank entries during validation or preserve the explicit policy as deny-all when none resolve.
Useful? React with 👍 / 👎.
| return acc; | ||
| }, {}); | ||
| const allowed = Object.keys(cfg.agents?.defaults?.models ?? {}); | ||
| const allowed = [...resolveConfiguredModelPolicyAllow({ cfg, agentId: workspaceAgentId }).refs]; |
There was a problem hiding this comment.
When openclaw models status --agent research reads a per-agent policy such as allow: ["approved"], where approved exists only in that agent's models map, this line adds the alias to allowed but the alias index below is built without workspaceAgentId. resolveStatusModelRef therefore interprets it as DEFAULT_PROVIDER/approved, causing the auth overview to inspect the wrong provider and potentially omit missing auth for the actual allowed model. Build the status alias index with the selected agent id.
Useful? React with 👍 / 👎.
afc6e60 to
13bf533
Compare
|
/allow-security-sensitive-change 13bf533 |
|
/allow-dependencies-change 13bf533 |
clawsweeper
Bot
added
the
rating: 🦪 silver shellfish
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13bf53398d
ℹ️ 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".
| catalog: params.catalog, | ||
| defaultProvider: params.defaultProvider, | ||
| defaultModel: params.defaultModel, | ||
| agentId: params.agentId, |
There was a problem hiding this comment.
When an agent's modelPolicy.allow references an alias defined only in that agent's models map, this newly agent-scoped policy rejects the same alias through session_status(model=...): src/agents/tools/session-status-tool.ts:461-464 builds its parsing alias index without params.agentId, while the policy at lines 474-483 now resolves the alias in the agent scope. The input is therefore parsed as <current-provider>/<alias> and fails policy.allowsKey instead of selecting the policy-approved target; pass the agent id to the tool's alias index as well.
Useful? React with 👍 / 👎.
13bf533 to
c8a8048
Compare
|
/allow-security-sensitive-change c8a8048 |
|
/allow-dependencies-change c8a8048 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8a80486b1
ℹ️ 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".
| if (hasExplicitModelPolicyAllow(agentPolicy)) { | ||
| return { | ||
| refs: agentPolicy?.allow ?? [], | ||
| configPath: AGENT_MODEL_POLICY_ALLOW_CONFIG_PATH, | ||
| repairConfigPath: AGENT_MODEL_POLICY_ALLOW_CONFIG_PATH, |
There was a problem hiding this comment.
When a per-agent policy allows an agent-local alias such as modelPolicy.allow: ["approved"], OpenAI-compatible chat/responses requests using x-openclaw-model: approved are rejected even though /model approved is accepted. src/gateway/http-utils.ts:147-166 parses the header directly with parseModelRef, so it checks <default-provider>/approved against the newly agent-scoped policy instead of resolving approved through that agent's alias index; resolve the header with the same agent-scoped alias semantics before applying policy.allowsKey.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
1 participant
What Problem This Solves
agents.defaults.modelsdid three jobs at once: per-model config (aliases/params), default/fallback selection, AND the--modeloverride allowlist. Because an EMPTY map meant allow-any and ANY entry meant a closed allowlist, the two most innocent commands —models set <default>andmodels aliases add <name> <ref>— silently flipped an agent from "any model allowed" to "locked down", so a previously-working--model openai/gpt-5.6-solstarted failing with "not allowed for agent", with nothing explaining why. Found by the R5 QA campaign.Why This Change Was Made
The allowlist should be an explicit, opt-in restriction — not a side effect of naming an alias or picking a default. This separates the three concerns and makes the restriction its own config surface, while preserving existing installations' behavior across upgrade.
User Impact
agents.defaults.modelPolicy.allow(and per-agentagents.list[].modelPolicy.allow) owns model-override restriction. Absent/empty ⇒ allow-any.agents.defaults.modelsis now metadata + default/fallback ordering only and no longer restricts overrides.models set/aliases addnever lock out overrides.openclaw doctor --fix(and the first config write) stampmodelPolicy.allowfrom an existing effectiveagents.defaults.modelsrestriction and set a migration marker, so restricted installs stay restricted. A sharedcomputeModelPolicyAllowlisthelper is used by runtime, doctor, and the config writer so pre- and post-migration behavior is identical.agents.defaults.modelsis a legacy allowlist — per-agentmodelsmaps are metadata-only and never become a restriction.modelPolicy.allow(not the legacy map) for repair. Configured fallbacks are failover-only and are not selectable as explicit overrides unless explicitly allowed. An explicitmodelPolicy: {}is an allow-any opt-in and is honored consistently across runtime, write, and cron/agent-scope inheritance.Evidence
models aliases add+models set,--model openai/gpt-5.6-solsucceeds (the original QA trap is gone); settingmodelPolicy.allowto clawrouter-only then rejects openai with a message naming the key; a live legacy-configdoctor --fixstampsmodelPolicy.allowfrom the legacy map and validates.{}semantics across all boundaries.git diff --numstatshows the per-agent legacy path was deleted, not added.This is a compatibility-sensitive config-surface change (new
modelPolicy.allowkey + doctor migration); existing restricted configs are preserved via marker-gated migration.