Fix stale model provider API config recovery

[obviyus]

Summary

• Migrate legacy models.providers.*.api = "openai" values to openai-completions in doctor/runtime compat.
• Let gateway startup skip providers with stale provider API enum values instead of aborting boot.

Tests

• pnpm test src/commands/doctor-legacy-config.migrations.test.ts src/gateway/server-startup-config.recovery.test.ts
• pnpm check:changed
• Temp gateway smoke with invalid provider API reached ready after logging the skipped provider.

Fixes #72477

[aisle-research-bot]

🔒 Aisle Security Analysis

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

#	Severity	Title
1	🟡 Medium	Log/CRLF injection via unsanitized model provider ID in startup warning
2	🟡 Medium	Prototype pollution risk when cloning config model providers via object spread
3	🟡 Medium	Prototype pollution risk in legacy config normalizers when copying provider maps via object spread

1. 🟡 Log/CRLF injection via unsanitized model provider ID in startup warning

Property	Value
Severity	Medium
CWE	CWE-117
Location	src/gateway/server-startup-config.ts:137-141

Description

resolveGatewayStartupConfigWithoutInvalidModelProviders logs providerId derived from snapshot.issues[].path (which is built from config object keys) without sanitization.

• models.providers keys are defined as z.record(z.string(), ModelProviderSchema) and therefore can contain arbitrary characters, including newlines (\n/\r) and terminal escape sequences.
• providerId is extracted from issue.path using a regex capture and then interpolated directly into a warning log line.
• If a crafted config contains a provider key with control characters, this can result in log forging/CRLF injection or terminal escape injection when logs are viewed.

Vulnerable code:

params.log.warn(
  `gateway: skipped model provider ${providerId}; configured provider api is invalid. ...`,
);

Recommendation

Sanitize untrusted strings before logging.

Use the existing terminal/log sanitizers (e.g. sanitizeTerminalText or sanitizeForLog) when interpolating providerId:

import { sanitizeTerminalText } from "../terminal/safe-text.js";

for (const providerId of providerIds) {
  const safeProviderId = sanitizeTerminalText(providerId);
  params.log.warn(
    `gateway: skipped model provider ${safeProviderId}; configured provider api is invalid. ` +
      `Run "openclaw doctor --fix" to repair the config.`,
  );
}

Optionally, also enforce a safe provider-id character set at schema validation time (e.g., ^[a-z0-9_-]+$) to prevent ambiguous paths/log output.

2. 🟡 Prototype pollution risk when cloning config model providers via object spread

Property	Value
Severity	Medium
CWE	CWE-1321
Location	src/gateway/server-startup-config.ts:84-95

Description

The gateway startup path attempts to recover from invalid models.providers.*.api values by cloning the models.providers object using object spread.

If a user-controlled config file contains a provider id key such as "__proto__", "constructor", or "prototype", spreading into a normal object can invoke the legacy __proto__ setter and mutate the prototype of the newly created object (nextProviders). Deleting the key afterwards does not revert the polluted prototype.

This occurs before the pruned config is revalidated (validateConfigObjectWithPlugins), so validation does not mitigate the pollution that can already have happened.

Vulnerable code:

const nextProviders = { ...providers };

Impact depends on downstream use of the polluted object, but prototype pollution can lead to unexpected property resolution across the process and, in some cases, security-sensitive logic bypasses.

Recommendation

Avoid spreading untrusted objects into normal {} objects. Instead, create a null-prototype object and/or explicitly skip blocked keys (__proto__, constructor, prototype).

Example fix:

import { isBlockedObjectKey } from "../config/prototype-keys.js";

const nextProviders: Record<string, unknown> = Object.create(null);
for (const [key, value] of Object.entries(providers)) {
  if (isBlockedObjectKey(key)) continue;
  nextProviders[key] = value;
}

Alternatively:

const nextProviders = Object.assign(Object.create(null), providers);

Then perform deletions on nextProviders as before.

3. 🟡 Prototype pollution risk in legacy config normalizers when copying provider maps via object spread

Property	Value
Severity	Medium
CWE	CWE-1321
Location	src/commands/doctor/shared/legacy-config-core-normalizers.ts:459-469

Description

Legacy config normalization copies the models.providers object (and each provider object) using object spread.

If a config contains provider IDs (object keys) like "__proto__" / "constructor" / "prototype", spreading rawProviders into {} can mutate the prototype of nextProviders. This can happen during openclaw doctor --fix flows on attacker-supplied config content.

Vulnerable code:

const nextProviders: Record<string, unknown> = { ...rawProviders };

Similar risk applies when spreading rawProvider into nextProvider if rawProvider contains dangerous keys.

Recommendation

When cloning config objects that may originate from untrusted/hand-edited files, avoid { ...obj } into plain objects.

Use a null-prototype target and skip blocked keys:

import { isBlockedObjectKey } from "../../../config/prototype-keys.js";

const nextProviders: Record<string, unknown> = Object.create(null);
for (const [k, v] of Object.entries(rawProviders)) {
  if (isBlockedObjectKey(k)) continue;
  nextProviders[k] = v;
}

const nextProvider: Record<string, unknown> = Object.create(null);
for (const [k, v] of Object.entries(rawProvider)) {
  if (isBlockedObjectKey(k)) continue;
  nextProvider[k] = v;
}

This prevents __proto__-setter based prototype mutation.

---

Analyzed PR: #72542 at commit 6cecdfa

_{Last updated on: 2026-04-27T03:44:04Z}

[greptile-apps]

Greptile Summary

This PR addresses stale models.providers.*.api = "openai" config values left over from a legacy enum rename. It adds a doctor normalizer to migrate them to "openai-completions" and introduces a gateway startup recovery path that skips individual providers whose api field contains an unrecognised enum value rather than aborting the entire boot sequence.

Confidence Score: 4/5

Safe to merge; logic is conservative and well-tested, with one minor P2 observation about the allowedValues guard.

No P0/P1 issues found. The only finding is a P2 style concern: the allowedValues guard in resolveInvalidModelProviderApiIssueProviderId is effectively a no-op due to the tight coupling between MODEL_APIS and the Zod schema, and it has no test coverage for the non-undefined path. All other logic (normalizer, pruning, re-validation, auth persistence flag) is correct and consistently handles edge cases.

src/gateway/server-startup-config.ts — specifically the allowedValues guard at line 77.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/gateway/server-startup-config.ts
Line: 77-78

Comment:
**`allowedValues` guard can silently bypass the recovery path**

The condition `!issue.allowedValues.every((value) => MODEL_API_VALUES.has(value))` returns `null` (preventing graceful skip) if the validator's `allowedValues` list contains any value not present in `MODEL_API_VALUES`. Because `ModelApiSchema` is defined as `z.enum(MODEL_APIS)` and `MODEL_API_VALUES` is built from the same `MODEL_APIS` constant, the two sets are always identical at compile time. In practice this guard is a no-op today, but if `allowedValues` is ever populated from a different code path (e.g., a version of the validator with newer enum entries loaded at runtime), the recovery silently falls back to last-known-good instead of skipping the provider. The test confirms this risk by omitting `allowedValues` entirely from the mock issues, so the guard has no test coverage. Consider documenting the invariant or removing the guard if it adds no safety value.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "docs(changelog): note model provider api..." | Re-trigger Greptile}

[obviyus]

Landed on main.

• Landed source commit: 6cecdfa
• Merge commit: 34f81c6