perf: avoid broad registry enumeration for default models list

[shakkernerd]

What changed

Default openclaw models list now resolves configured/default model rows without enumerating the full model registry.

Before this change, the default list path still paid for broad registry work even though it only needed configured rows. That meant models list could spend several seconds in all-model discovery before printing a single configured/default model.

This PR keeps the broad registry path for models list --all, but narrows the default path to:

• resolve configured entries from config
• look up only those configured models with registry.find()
• check exact configured availability with registry.hasConfiguredAuth(model)
• preserve the existing configured-row fallback for missing or forward-compatible models

Before

Measured against origin/main after build:

node openclaw.mjs models list --json

Warm runs:

• 9.31s
• 8.24s

Local output:

{
  "count": 1,
  "models": [
    {
      "key": "openai/gpt-5.5"
    }
  ]
}

After

Measured on this branch after build:

node openclaw.mjs models list --json

Warm runs:

• 4.15s
• 2.97s

Local output remains the same configured row:

{
  "count": 1,
  "models": [
    {
      "key": "openai/gpt-5.5"
    }
  ]
}

Behavior notes

• models list --all still uses the broad registry path.
• Default models list still hydrates configured rows through the registry.
• Missing configured models still flow through the existing fallback path.
• This does not introduce new manifest/plugin catalog behavior.

Verification

• pnpm check:changed
• node openclaw.mjs models list --json

[aisle-research-bot]

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

#	Severity	Title
1	🟡 Medium	External control of agent directory path leads to arbitrary filesystem reads during models list

1. 🟡 External control of agent directory path leads to arbitrary filesystem reads during `models list`

Property	Value
Severity	Medium
CWE	CWE-73
Location	src/commands/models/list.registry-load.ts:48-52

Description

loadConfiguredListModelRegistry() resolves agentDir via resolveOpenClawAgentDir() (which honors environment variable overrides) and then loads auth.json / models.json from that directory.

• resolveOpenClawAgentDir() trusts OPENCLAW_AGENT_DIR / PI_CODING_AGENT_DIR without constraining it to the application state directory.
• discoverAuthStorage() / discoverModels() derive file paths using path.join(agentDir, "auth.json") and path.join(agentDir, "models.json").
• This means an attacker who can influence the environment of a privileged invocation (e.g., a wrapper script, CI job, service, or sudo with preserved env) can redirect the CLI to read configuration-like files from attacker-chosen locations.
• The call uses { readOnly: true } only to skip scrubLegacyStaticAuthJsonEntriesForDiscovery(), but it does not guarantee that the underlying auth storage/registry implementation will never create or modify those files.

Vulnerable code (newly introduced path usage):

const agentDir = resolveOpenClawAgentDir();
const authStorage = discoverAuthStorage(agentDir, { readOnly: true });
const registry = discoverModels(authStorage, agentDir, {
  providerFilter: opts?.providerFilter,
});

Related path source:

const override = env.OPENCLAW_AGENT_DIR?.trim() || env.PI_CODING_AGENT_DIR?.trim();
if (override) {
  return resolveUserPath(override, env);
}

Recommendation

Constrain and harden agent directory resolution when used for filesystem access:

• Validate and canonicalize agentDir with fs.realpathSync() and ensure it is within the expected state directory (e.g., $OPENCLAW_STATE_DIR/agents/...) unless an explicit --agent-dir flag is provided.
• If overrides are needed, require an explicit opt-in flag and/or warn when OPENCLAW_AGENT_DIR is set.
• For read-only operations like models list, use an in-memory auth storage or ensure the underlying AuthStorage implementation is opened in a true read-only mode (no creation/writes).

Example (conceptual):

import fs from "node:fs";
import path from "node:path";
import { resolveStateDir } from "../config/paths.js";

function resolveSafeAgentDir(env: NodeJS.ProcessEnv = process.env) {
  const raw = resolveOpenClawAgentDir(env);
  const real = fs.realpathSync(raw);
  const allowedRoot = fs.realpathSync(path.join(resolveStateDir(env), "agents"));
  if (!real.startsWith(allowedRoot + path.sep)) {
    throw new Error("Refusing to load agent data outside state directory");
  }
  return real;
}

Also consider rejecting non-directory paths and checking ownership/permissions to reduce symlink-based redirection risks.

---

Analyzed PR: #70883 at commit 37b1e67

_{Last updated on: 2026-04-24T02:29:25Z}

[greptile-apps]

Greptile Summary

This PR narrows the default models list path to avoid expensive broad registry enumeration — instead of calling getAll() + getAvailable() across the full model registry, it initializes the registry via discoverModels and then calls registry.find() only for configured entries. The --all flag retains the full enumeration path. Benchmarks show a 2–3× wall-time improvement on warm runs.

Confidence Score: 5/5

Safe to merge — the narrowed path is logically consistent with the broad path for the default case, all three key sets (discoveredKeys, availableKeys, registry) are populated correctly, and the test suite was updated to cover the new behavior.

No P0 or P1 findings. The semantics of discoveredKeys and availableKeys are preserved: loadConfiguredListModelRegistry builds them using the same registry.find() call that appendConfiguredRows uses, so allowProviderAvailabilityFallback is always consistent. The if (!registry) guard is now dead code for the non-–all path but is a harmless safety net.

No files require special attention.

_{Reviews (1): Last reviewed commit: "test: cover default models list registry..." | Re-trigger Greptile}

[shakkernerd]

Landed via temp rebase onto current main.

• Gate: pnpm test src/commands/models.list.e2e.test.ts src/commands/models/list.list-command.forward-compat.test.ts src/commands/models/list.rows.test.ts
• Gate: pnpm check
• Gate: pnpm build
• Gate: pnpm check:changed
• Smoke: node openclaw.mjs models list --json
• Smoke: node openclaw.mjs models list --all --provider moonshot --json
• Smoke: node openclaw.mjs models list --all --provider codex --json
• Rebased PR head before merge: 37b1e67
• Merge commit: 64ed439

Thanks @shakkernerd!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 84fa30f038

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Skip non-filtered entries before registry find calls

When models list runs in configured mode with --provider, this loop still calls registry.find(...) for every configured entry before provider filtering happens later in appendConfiguredRows. That means a lookup failure from an unrelated provider can now abort the command even though that provider would not appear in output. Before this change, filtered-out entries were skipped before any per-entry find call, so --provider was insulated from other configured providers' lookup failures.

Useful? React with 👍 / 👎.