fix(secrets): avoid re-materializing plaintext apiKey in agent models.json by kelvinCB · Pull Request #28393 · openclaw/openclaw

kelvinCB · 2026-02-27T06:27:38Z

Summary

When runtime secrets are active, loadConfig() may return a resolved runtime snapshot with plaintext provider keys. ensureOpenClawModelsJson() used that snapshot directly, which could persist plaintext API keys into ~/.openclaw/agents/*/agent/models.json.

What changed

Added getRuntimeConfigSourceSnapshot() export from config IO.
Updated ensureOpenClawModelsJson() to prefer the runtime source snapshot (pre-resolution config) when choosing explicit model providers.
- Falls back to regular config when source snapshot is unavailable.
Added regression test:
- sets a runtime resolved config containing plaintext apiKey
- sets runtime source config containing SecretRef
- verifies generated models.json keeps the SecretRef and does not persist plaintext.

Why this is safe

Behavior only changes when a runtime source snapshot exists.
Existing merge/normalize flows remain unchanged.
It narrows secret exposure on disk while preserving provider config semantics.

Validation

Ran: pnpm test src/agents/models-config.skips-writing-models-json-no-env-token.test.ts
Result: pass (5 tests)

…hot (openclaw#28359)

greptile-apps · 2026-02-27T06:31:38Z

Greptile Summary

Properly fixes the security issue where plaintext API keys could be persisted to ~/.openclaw/agents/*/agent/models.json when runtime secrets are active. The fix prefers the runtime source snapshot (containing SecretRef) over the resolved config (containing plaintext values) when writing provider configurations to disk.

Key Changes

Added getRuntimeConfigSourceSnapshot() export to access pre-resolution config
Updated ensureOpenClawModelsJson() to use source snapshot when available, falling back to resolved config only when source snapshot is unavailable
Added regression test validating that SecretRef is preserved and plaintext is not persisted

Technical Details

The solution correctly uses a fallback chain: sourceCfg?.models?.providers ?? cfg.models?.providers ?? {}, ensuring backwards compatibility when no runtime source snapshot exists. The test properly validates the fix using setRuntimeConfigSnapshot() with both resolved and source configs.

Note on Merge Mode

When mode is "merge" and an existing models.json contains string-based API keys, those will be preserved per the existing merge logic (lines 169-174 in models-config.ts). This is documented as intentional behavior to preserve user customizations.

Confidence Score: 4/5

This PR is safe to merge with minimal risk - it addresses a security issue without breaking existing functionality
The fix is well-implemented with proper fallback logic and test coverage. Score of 4 reflects high confidence with one documented limitation: merge mode preserves existing plaintext keys from disk, which is intentional per the PR description stating "Existing merge/normalize flows remain unchanged"
No files require special attention - all changes are straightforward and well-tested

_{Last reviewed commit: 3c7a7ec}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c7a7eca9f

ℹ️ 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".

chatgpt-codex-connector · 2026-02-27T06:32:48Z

src/agents/models-config.ts

+  // with plaintext apiKey values. Prefer the source snapshot (with SecretRef inputs)
+  // when available so models.json does not persist resolved secrets to disk.
+  const sourceCfg = getRuntimeConfigSourceSnapshot();
+  const explicitProviders = (sourceCfg?.models?.providers ?? cfg.models?.providers ?? {}) as Record<


Respect passed config when selecting explicit model providers

When a runtime snapshot is active, this now always prefers getRuntimeConfigSourceSnapshot() over the config argument, so callers that pass an explicit config object can no longer control models.providers. In gateway flows where ensureOpenClawModelsJson() is called with request-specific config (for example, code paths that pass params.cfg/effectiveCfg), provider additions or overrides in that config are silently ignored and models.json is generated from the global source snapshot instead, which can lead to missing/incorrect registry entries for that run.

Useful? React with 👍 / 👎.

…fig types

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 49cdf83073

ℹ️ 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".

chatgpt-codex-connector · 2026-02-27T06:49:31Z

src/agents/models-config.ts

+  const sourceCfg = getRuntimeConfigSourceSnapshot();
+  const explicitProviders = sourceCfg?.models?.providers ?? cfg.models?.providers ?? {};


Respect passed config before source snapshot providers

ensureOpenClawModelsJson() now always reads explicitProviders from getRuntimeConfigSourceSnapshot() when a runtime snapshot exists, even when a caller explicitly passes a config argument; this changes precedence and can silently drop per-call provider overrides/additions, producing incorrect models.json contents and model resolution failures in runtime-secret sessions. Fresh evidence: this function is called with caller-supplied configs in production paths such as runImagePrompt (src/agents/tools/image-tool.ts), describeImageWithModel (src/media-understanding/providers/image.ts), and runEmbeddedPiAgent (src/agents/pi-embedded-runner/run.ts).

Useful? React with 👍 / 👎.

openclaw-barnacle · 2026-03-05T04:07:51Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

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

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.

fix(secrets): keep SecretRef in models.json from runtime source snaps…

3c7a7ec

…hot (openclaw#28359)

openclaw-barnacle bot added agents Agent runtime and tooling size: S labels Feb 27, 2026

chatgpt-codex-connector bot reviewed Feb 27, 2026

View reviewed changes

kelvinCB added 2 commits February 26, 2026 22:41

test(secrets): align runtime-source regression fixture with model con…

0396d3a

…fig types

fix(lint): remove unnecessary provider type assertion in models config

49cdf83

chatgpt-codex-connector bot reviewed Feb 27, 2026

View reviewed changes

steipete mentioned this pull request Mar 3, 2026

security: redact resolved API keys from models.json #28645

Closed

openclaw-barnacle bot added the stale Marked as stale due to inactivity label Mar 5, 2026

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(secrets): avoid re-materializing plaintext apiKey in agent models.json#28393

fix(secrets): avoid re-materializing plaintext apiKey in agent models.json#28393
kelvinCB wants to merge 3 commits intoopenclaw:mainfrom
kelvinCB:fix/28359-no-plaintext-models-runtime

kelvinCB commented Feb 27, 2026

Uh oh!

greptile-apps bot commented Feb 27, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Uh oh!

openclaw-barnacle bot commented Mar 5, 2026

Uh oh!

joshavant commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		const sourceCfg = getRuntimeConfigSourceSnapshot();
		const explicitProviders = sourceCfg?.models?.providers ?? cfg.models?.providers ?? {};

Uh oh!

Conversation

kelvinCB commented Feb 27, 2026

Summary

What changed

Why this is safe

Validation

Uh oh!

greptile-apps bot commented Feb 27, 2026

Greptile Summary

Key Changes

Technical Details

Note on Merge Mode

Confidence Score: 4/5

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

openclaw-barnacle bot commented Mar 5, 2026

Uh oh!

joshavant commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants