fix(security): redact resolved secrets from models.json by Bartok9 · Pull Request #37568 · openclaw/openclaw

Bartok9 · 2026-03-06T09:37:46Z

Summary

When the gateway resolves secrets at startup (env vars, SecretRef with Vault/exec/file providers), the resolved plaintext values were being written to agents/*/agent/models.json files on disk. This undermined the security benefits of centralized secret management.

Changes

Added isEnvVarNameOrMarker() helper to distinguish between env var names and resolved secrets
Added redactResolvedSecrets() function that strips resolved secrets while preserving env var names
Applied redaction before writing to models.json in ensureOpenClawModelsJson()

What gets preserved

Env var names: OPENAI_API_KEY, ANTHROPIC_API_KEY, AWS_ACCESS_KEY_ID, etc. (UPPER_SNAKE_CASE)
Special markers: ollama-local, aws-sdk

What gets redacted

Resolved secret values that don't match the env var pattern (e.g., sk-1234..., xai-abc...)

Testing

Added comprehensive test suite in models-config.redact-secrets.test.ts
All 76 existing models-config tests pass
Verified file permissions remain at 0o600

Fixes #37512

When the gateway resolves secrets at startup (env vars, SecretRef with Vault/exec/file providers), the resolved plaintext values were being written to agents/*/agent/models.json files on disk. This undermined the security benefits of centralized secret management. This change adds a redaction step before writing to models.json that: - Preserves env var names (UPPER_SNAKE_CASE patterns like OPENAI_API_KEY) - Preserves special markers (ollama-local, aws-sdk) - Strips resolved secret values that look like actual API keys The file permissions on models.json are already set to 0o600 (owner-only), but this change ensures secrets don't end up on disk at all. Fixes openclaw#37512

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f66d33d10

ℹ️ 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 · 2026-03-06T09:44:00Z

src/agents/models-config.ts

+  if (trimmed === "ollama-local") {
+    return true;


Preserve OAuth placeholder apiKeys during redaction

The new marker allowlist only preserves ollama-local/aws-sdk, so providers that intentionally use non-secret placeholder strings (for example minimax-oauth and qwen-oauth) get stripped as if they were resolved secrets. resolveImplicitProviders() sets those placeholders for minimax-portal and qwen-portal (src/agents/models-config.providers.ts:65, src/agents/models-config.providers.ts:135, src/agents/models-config.providers.ts:943, src/agents/models-config.providers.ts:979), and the same module notes that pi’s ModelRegistry requires apiKey when providers define models (src/agents/models-config.providers.ts:521-543), so this redaction can remove OAuth providers from models.json and make their models unresolvable.

Useful? React with 👍 / 👎.

greptile-apps · 2026-03-06T09:46:32Z

Greptile Summary

This PR introduces a security fix that redacts resolved plaintext API keys from agents/*/agent/models.json before writing to disk, ensuring only env var name references (e.g. OPENAI_API_KEY) and a small set of special markers (e.g. ollama-local, aws-sdk) are persisted rather than actual secret values.

Key changes:

isEnvVarNameOrMarker() — identifies env var names (UPPER_SNAKE_CASE regex) and a hard-coded allow-list of special synthetic markers.
redactResolvedSecrets() — strips the apiKey field from any provider whose value does not match the above check.
Applied in ensureOpenClawModelsJson() immediately before the JSON serialisation step.
New test file models-config.redact-secrets.test.ts covering env var preservation, secret stripping, the ollama-local marker, and file permission verification.

Critical issue found:

The isEnvVarNameOrMarker() allow-list is incomplete. Two OAuth placeholder markers used by implicit providers are not included:

"minimax-oauth" — assigned as apiKey for the minimax-portal provider
"qwen-oauth" — assigned as apiKey for the qwen-portal provider

Both values contain lowercase letters and hyphens, so they fail the UPPER_SNAKE_CASE regex check and are not listed as special cases. As a result, redactResolvedSecrets() will strip them from the written models.json, silently breaking OAuth authentication for these two providers after this PR lands. This is a functional regression, not just a theoretical risk. The omission is compounded by missing test coverage for these OAuth placeholder values, which would have caught the gap before merge.

Confidence Score: 1/5

Not safe to merge as-is. The incomplete marker allow-list will silently break OAuth authentication for MiniMax Portal and Qwen Portal providers.
The PR introduces a critical logic bug: two OAuth placeholder values ("minimax-oauth" and "qwen-oauth") used by implicit providers will be stripped from the redacted models.json because they are missing from the isEnvVarNameOrMarker() allow-list. At runtime, reading the persisted config will find no apiKey field, silently failing OAuth flows. This is a functional regression that directly contradicts the PR's security intent. Missing test coverage for these placeholders enabled the bug to slip through. The implementation approach is sound, but the allow-list must be extended before merge.
src/agents/models-config.ts — the isEnvVarNameOrMarker() function needs the two OAuth placeholder constant values ("minimax-oauth" and "qwen-oauth") added to its explicit allow-list. Test coverage for these markers should also be added to src/agents/models-config.redact-secrets.test.ts.

_{Last reviewed commit: 9f66d33}

greptile-apps · 2026-03-06T09:46:36Z

src/agents/models-config.ts

+function isEnvVarNameOrMarker(value: string): boolean {
+  const trimmed = value.trim();
+  if (!trimmed) {
+    return false;
+  }
+
+  // Synthetic local provider markers (e.g., "ollama-local")
+  if (trimmed === "ollama-local") {
+    return true;
+  }
+
+  // AWS SDK auth marker
+  if (trimmed === "aws-sdk") {
+    return true;
+  }
+
+  // Environment variable name pattern: UPPER_SNAKE_CASE
+  // Must start with letter, contain only uppercase letters, digits, underscores
+  // Examples: OPENAI_API_KEY, ANTHROPIC_API_KEY, AWS_ACCESS_KEY_ID
+  return /^[A-Z][A-Z0-9_]*$/.test(trimmed);
+}


Missing OAuth placeholder markers in allow-list

isEnvVarNameOrMarker() only recognises "ollama-local" and "aws-sdk" as special markers, but models-config.providers.ts defines two OAuth placeholder values that are assigned to apiKey fields at runtime:

MINIMAX_OAUTH_PLACEHOLDER — the string constant "minimax-oauth" (line 65) assigned as apiKey for the minimax-portal provider

QWEN_PORTAL_OAUTH_PLACEHOLDER — the string constant "qwen-oauth" (line 135) assigned as apiKey for the qwen-portal provider

Both constants contain lowercase letters and a hyphen, so they do not match ^[A-Z][A-Z0-9_]*$ and are not listed as special markers. As a result, redactResolvedSecrets() will strip them from the written models.json.

At runtime the system reads apiKey back from models.json to determine the auth mode. If the field is absent, OAuth flows for MiniMax Portal and Qwen Portal will silently fail.

Both constant values should be added to isEnvVarNameOrMarker() alongside "ollama-local" and "aws-sdk". A more future-proof approach would be to accept an explicit allow-list of known marker strings passed in from the call site, so new providers don't require changes in two separate places.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/models-config.ts Line: 27-47 Comment: **Missing OAuth placeholder markers in allow-list** `isEnvVarNameOrMarker()` only recognises `"ollama-local"` and `"aws-sdk"` as special markers, but `models-config.providers.ts` defines two OAuth placeholder values that are assigned to `apiKey` fields at runtime: - `MINIMAX_OAUTH_PLACEHOLDER` — the string constant `"minimax-oauth"` (line 65) assigned as `apiKey` for the `minimax-portal` provider - `QWEN_PORTAL_OAUTH_PLACEHOLDER` — the string constant `"qwen-oauth"` (line 135) assigned as `apiKey` for the `qwen-portal` provider Both constants contain lowercase letters and a hyphen, so they do **not** match `^[A-Z][A-Z0-9_]*$` and are **not** listed as special markers. As a result, `redactResolvedSecrets()` will strip them from the written `models.json`. At runtime the system reads `apiKey` back from `models.json` to determine the auth mode. If the field is absent, OAuth flows for MiniMax Portal and Qwen Portal will silently fail. Both constant values should be added to `isEnvVarNameOrMarker()` alongside `"ollama-local"` and `"aws-sdk"`. A more future-proof approach would be to accept an explicit allow-list of known marker strings passed in from the call site, so new providers don't require changes in two separate places. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-03-06T09:46:37Z

src/agents/models-config.redact-secrets.test.ts

+  it("preserves special marker values like ollama-local", async () => {
+    const config = {
+      models: {
+        providers: {
+          ollama: {
+            apiKey: "ollama-local",
+            baseUrl: "http://localhost:11434",
+            models: [{ id: "llama3" }],
+          },
+        },
+      },
+    };
+
+    await ensureOpenClawModelsJson(config, agentDir);
+
+    const modelsJson = await fs.readFile(path.join(agentDir, "models.json"), "utf8");
+    const parsed = JSON.parse(modelsJson);
+
+    expect(parsed.providers.ollama?.apiKey).toBe("ollama-local");
+  });


Missing test coverage for OAuth placeholder markers

The test suite covers "ollama-local" but has no cases for the minimax-portal and qwen-portal OAuth placeholder apiKey values—the constants "minimax-oauth" and "qwen-oauth" from models-config.providers.ts (lines 65 and 135). Those values contain lowercase + hyphen characters that do not match the UPPER_SNAKE_CASE regex, so they fall into the same bucket as resolved secrets and will be stripped. A test mirroring the ollama-local structure would have caught this gap before the bug shipped.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/models-config.redact-secrets.test.ts Line: 95-114 Comment: **Missing test coverage for OAuth placeholder markers** The test suite covers `"ollama-local"` but has no cases for the `minimax-portal` and `qwen-portal` OAuth placeholder `apiKey` values—the constants `"minimax-oauth"` and `"qwen-oauth"` from `models-config.providers.ts` (lines 65 and 135). Those values contain lowercase + hyphen characters that do not match the UPPER_SNAKE_CASE regex, so they fall into the same bucket as resolved secrets and will be stripped. A test mirroring the `ollama-local` structure would have caught this gap before the bug shipped. How can I resolve this? If you propose a fix, please make it concise.

joshavant · 2026-03-07T17:32:54Z

Thanks for the work on this PR and for contributing to this fix path.

Closing this as superseded by #38955 (merged), which integrated the relevant fixes from this thread and completed the broader hardening scope for the secrets/models persistence cluster.

The merged work now covers source-snapshot persistence, marker/provenance auth handling, audit/apply expansion (including models.json/header residues), and write-mode hardening/tests/docs in one end-to-end fix.

Appreciate the contribution here. If there’s a remaining unique change from this PR that should still land, please call it out in a follow-up issue and reference #38955.

openclaw-barnacle bot added agents Agent runtime and tooling size: M labels Mar 6, 2026

chatgpt-codex-connector bot reviewed Mar 6, 2026

View reviewed changes

greptile-apps bot reviewed Mar 6, 2026

View reviewed changes

This comment was marked as spam.

Sign in to view

This was referenced Mar 7, 2026

Models/Secrets: harden SecretRef-managed models.json persistence and audit #38470

Closed

Secrets: harden SecretRef-safe models.json persistence #38955

Merged

joshavant closed this Mar 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(security): redact resolved secrets from models.json#37568

fix(security): redact resolved secrets from models.json#37568
Bartok9 wants to merge 1 commit intoopenclaw:mainfrom
Bartok9:fix/37512-redact-models-json-secrets

Bartok9 commented Mar 6, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 6, 2026

Uh oh!

greptile-apps bot commented Mar 6, 2026

Uh oh!

greptile-apps bot Mar 6, 2026

Uh oh!

greptile-apps bot Mar 6, 2026

Uh oh!

This comment was marked as spam.

This comment was marked as spam.

joshavant commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Bartok9 commented Mar 6, 2026

Summary

Changes

What gets preserved

What gets redacted

Testing

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Mar 6, 2026

Greptile Summary

Confidence Score: 1/5

Uh oh!

greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

This comment was marked as spam.

joshavant commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants