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

src/agents/models-config.ts

@@ -1,9 +1,15 @@
 import fs from "node:fs/promises";
 import path from "node:path";
-import { type OpenClawConfig, loadConfig } from "../config/config.js";
+import {
+  getRuntimeConfigSnapshot,
+  getRuntimeConfigSourceSnapshot,
+  type OpenClawConfig,
+  loadConfig,
+} from "../config/config.js";
 import { applyConfigEnvVars } from "../config/env-vars.js";
 import { isRecord } from "../utils.js";
 import { resolveOpenClawAgentDir } from "./agent-paths.js";
+import { isNonSecretApiKeyMarker } from "./model-auth-markers.js";
 import {
   normalizeProviders,
   type ProviderConfig,
@@ -15,6 +21,7 @@ import {
 type ModelsConfig = NonNullable<OpenClawConfig["models"]>;
 
 const DEFAULT_MODE: NonNullable<ModelsConfig["mode"]> = "merge";
+const MODELS_JSON_WRITE_LOCKS = new Map<string, Promise<void>>();
 
 function resolvePreferredTokenLimit(explicitValue: number, implicitValue: number): number {
   // Keep catalog refresh behavior for stale low values while preserving
@@ -141,8 +148,9 @@ async function resolveProvidersForModelsJson(params: {
 function mergeWithExistingProviderSecrets(params: {
   nextProviders: Record<string, ProviderConfig>;
   existingProviders: Record<string, NonNullable<ModelsConfig["providers"]>[string]>;
+  secretRefManagedProviders: ReadonlySet<string>;
 }): Record<string, ProviderConfig> {
-  const { nextProviders, existingProviders } = params;
+  const { nextProviders, existingProviders, secretRefManagedProviders } = params;
   const mergedProviders: Record<string, ProviderConfig> = {};
   for (const [key, entry] of Object.entries(existingProviders)) {
     mergedProviders[key] = entry;
@@ -159,7 +167,12 @@ function mergeWithExistingProviderSecrets(params: {
       continue;
     }
     const preserved: Record<string, unknown> = {};
-    if (typeof existing.apiKey === "string" && existing.apiKey) {
+    if (
+      !secretRefManagedProviders.has(key) &&
+      typeof existing.apiKey === "string" &&
+      existing.apiKey &&
+      !isNonSecretApiKeyMarker(existing.apiKey, { includeEnvVarName: false })
+    ) {
       preserved.apiKey = existing.apiKey;
     }
     if (typeof existing.baseUrl === "string" && existing.baseUrl) {
@@ -174,6 +187,7 @@ async function resolveProvidersForMode(params: {
   mode: NonNullable<ModelsConfig["mode"]>;
   targetPath: string;
   providers: Record<string, ProviderConfig>;
+  secretRefManagedProviders: ReadonlySet<string>;
 }): Promise<Record<string, ProviderConfig>> {
   if (params.mode !== "merge") {
     return params.providers;
@@ -189,6 +203,7 @@ async function resolveProvidersForMode(params: {
   return mergeWithExistingProviderSecrets({
     nextProviders: params.providers,
     existingProviders,
+    secretRefManagedProviders: params.secretRefManagedProviders,
   });
 }
 
@@ -200,45 +215,94 @@ async function readRawFile(pathname: string): Promise<string> {
   }
 }
 
+async function ensureModelsFileMode(pathname: string): Promise<void> {
+  await fs.chmod(pathname, 0o600).catch(() => {
+    // best-effort
+  });
+}
+
+function resolveModelsConfigInput(config?: OpenClawConfig): OpenClawConfig {
+  const runtimeSource = getRuntimeConfigSourceSnapshot();
+  if (!runtimeSource) {
+    return config ?? loadConfig();
+  }
+  if (!config) {
+    return runtimeSource;
+  }
+  const runtimeResolved = getRuntimeConfigSnapshot();
+  if (runtimeResolved && config === runtimeResolved) {
+    return runtimeSource;
+  }
+  return config;
+}
+
+async function withModelsJsonWriteLock<T>(targetPath: string, run: () => Promise<T>): Promise<T> {
+  const prior = MODELS_JSON_WRITE_LOCKS.get(targetPath) ?? Promise.resolve();
+  let release: () => void = () => {};
+  const gate = new Promise<void>((resolve) => {
+    release = resolve;
+  });
+  const pending = prior.then(() => gate);
+  MODELS_JSON_WRITE_LOCKS.set(targetPath, pending);
+  try {
+    await prior;
+    return await run();
+  } finally {
+    release();
+    if (MODELS_JSON_WRITE_LOCKS.get(targetPath) === pending) {
+      MODELS_JSON_WRITE_LOCKS.delete(targetPath);
+    }
+  }
+}
+
 export async function ensureOpenClawModelsJson(
   config?: OpenClawConfig,
   agentDirOverride?: string,
 ): Promise<{ agentDir: string; wrote: boolean }> {
-  const cfg = config ?? loadConfig();
+  const cfg = resolveModelsConfigInput(config);
   const agentDir = agentDirOverride?.trim() ? agentDirOverride.trim() : resolveOpenClawAgentDir();
-
-  // Ensure config env vars (e.g. AWS_PROFILE, AWS_ACCESS_KEY_ID) are
-  // available in process.env before implicit provider discovery.  Some
-  // callers (agent runner, tools) pass config objects that haven't gone
-  // through the full loadConfig() pipeline which applies these.
-  applyConfigEnvVars(cfg);
-
-  const providers = await resolveProvidersForModelsJson({ cfg, agentDir });
-
-  if (Object.keys(providers).length === 0) {
-    return { agentDir, wrote: false };
-  }
-
-  const mode = cfg.models?.mode ?? DEFAULT_MODE;
   const targetPath = path.join(agentDir, "models.json");
-  const mergedProviders = await resolveProvidersForMode({
-    mode,
-    targetPath,
-    providers,
+
+  return await withModelsJsonWriteLock(targetPath, async () => {
+    // Ensure config env vars (e.g. AWS_PROFILE, AWS_ACCESS_KEY_ID) are
+    // available in process.env before implicit provider discovery. Some
+    // callers (agent runner, tools) pass config objects that haven't gone
+    // through the full loadConfig() pipeline which applies these.
+    applyConfigEnvVars(cfg);
+
+    const providers = await resolveProvidersForModelsJson({ cfg, agentDir });
+
+    if (Object.keys(providers).length === 0) {
+      return { agentDir, wrote: false };
+    }
+
+    const mode = cfg.models?.mode ?? DEFAULT_MODE;
+    const secretRefManagedProviders = new Set<string>();
+
+    const normalizedProviders =
+      normalizeProviders({
+        providers,
+        agentDir,
+        secretDefaults: cfg.secrets?.defaults,
+        secretRefManagedProviders,
+      }) ?? providers;
+    const mergedProviders = await resolveProvidersForMode({
+      mode,
+      targetPath,
+      providers: normalizedProviders,
+      secretRefManagedProviders,
+    });
+    const next = `${JSON.stringify({ providers: mergedProviders }, null, 2)}\n`;
+    const existingRaw = await readRawFile(targetPath);
+
+    if (existingRaw === next) {
+      await ensureModelsFileMode(targetPath);
+      return { agentDir, wrote: false };
+    }
+
+    await fs.mkdir(agentDir, { recursive: true, mode: 0o700 });
+    await fs.writeFile(targetPath, next, { mode: 0o600 });
+    await ensureModelsFileMode(targetPath);
+    return { agentDir, wrote: true };
   });
-
-  const normalizedProviders = normalizeProviders({
-    providers: mergedProviders,
-    agentDir,
-  });
-  const next = `${JSON.stringify({ providers: normalizedProviders }, null, 2)}\n`;
-  const existingRaw = await readRawFile(targetPath);
-
-  if (existingRaw === next) {
-    return { agentDir, wrote: false };
-  }
-
-  await fs.mkdir(agentDir, { recursive: true, mode: 0o700 });
-  await fs.writeFile(targetPath, next, { mode: 0o600 });
-  return { agentDir, wrote: true };
 }