diff --git a/src/agents/auth-profiles/store.ts b/src/agents/auth-profiles/store.ts index 389a4f81f..2d64cdc8c 100644 --- a/src/agents/auth-profiles/store.ts +++ b/src/agents/auth-profiles/store.ts @@ -418,7 +418,8 @@ function loadAuthProfileStoreForAgent( const mergedOAuth = mergeOAuthFileIntoStore(store); // Keep external CLI credentials visible in runtime even during read-only loads. const syncedCli = syncExternalCliCredentials(store); - const shouldWrite = !readOnly && (legacy !== null || mergedOAuth || syncedCli); + const forceReadOnly = process.env.OPENCLAW_AUTH_STORE_READONLY === "1"; + const shouldWrite = !readOnly && !forceReadOnly && (legacy !== null || mergedOAuth || syncedCli); if (shouldWrite) { saveJsonFile(authPath, store); } diff --git a/src/agents/pi-model-discovery.auth.test.ts b/src/agents/pi-model-discovery.auth.test.ts index b96fb9ab8..0804ed423 100644 --- a/src/agents/pi-model-discovery.auth.test.ts +++ b/src/agents/pi-model-discovery.auth.test.ts @@ -113,4 +113,49 @@ describe("discoverAuthStorage", () => { await fs.rm(agentDir, { recursive: true, force: true }); } }); + + it("preserves legacy auth.json when auth store is forced read-only", async () => { + const agentDir = await createAgentDir(); + const previous = process.env.OPENCLAW_AUTH_STORE_READONLY; + process.env.OPENCLAW_AUTH_STORE_READONLY = "1"; + try { + saveAuthProfileStore( + { + version: 1, + profiles: { + "openrouter:default": { + type: "api_key", + provider: "openrouter", + key: "sk-or-v1-runtime", + }, + }, + }, + agentDir, + ); + await fs.writeFile( + path.join(agentDir, "auth.json"), + JSON.stringify( + { + openrouter: { type: "api_key", key: "legacy-static-key" }, + }, + null, + 2, + ), + ); + + discoverAuthStorage(agentDir); + + const parsed = JSON.parse(await fs.readFile(path.join(agentDir, "auth.json"), "utf8")) as { + [key: string]: unknown; + }; + expect(parsed.openrouter).toMatchObject({ type: "api_key", key: "legacy-static-key" }); + } finally { + if (previous === undefined) { + delete process.env.OPENCLAW_AUTH_STORE_READONLY; + } else { + process.env.OPENCLAW_AUTH_STORE_READONLY = previous; + } + await fs.rm(agentDir, { recursive: true, force: true }); + } + }); }); diff --git a/src/agents/pi-model-discovery.ts b/src/agents/pi-model-discovery.ts index dc8463b6d..a2d3dccf6 100644 --- a/src/agents/pi-model-discovery.ts +++ b/src/agents/pi-model-discovery.ts @@ -15,6 +15,9 @@ function isRecord(value: unknown): value is Record { } function scrubLegacyStaticAuthJsonEntries(pathname: string): void { + if (process.env.OPENCLAW_AUTH_STORE_READONLY === "1") { + return; + } if (!fs.existsSync(pathname)) { return; } diff --git a/src/entry.ts b/src/entry.ts index 92bd00640..6f664edce 100644 --- a/src/entry.ts +++ b/src/entry.ts @@ -15,6 +15,16 @@ const ENTRY_WRAPPER_PAIRS = [ { wrapperBasename: "openclaw.js", entryBasename: "entry.js" }, ] as const; +function shouldForceReadOnlyAuthStore(argv: string[]): boolean { + const tokens = argv.slice(2).filter((token) => token.length > 0 && !token.startsWith("-")); + for (let index = 0; index < tokens.length - 1; index += 1) { + if (tokens[index] === "secrets" && tokens[index + 1] === "audit") { + return true; + } + } + return false; +} + // Guard: only run entry-point logic when this file is the main module. // The bundler may import entry.js as a shared dependency when dist/index.js // is the actual entry point; without this guard the top-level code below @@ -32,6 +42,10 @@ if ( installProcessWarningFilter(); normalizeEnv(); + if (shouldForceReadOnlyAuthStore(process.argv)) { + process.env.OPENCLAW_AUTH_STORE_READONLY = "1"; + } + if (process.argv.includes("--no-color")) { process.env.NO_COLOR = "1"; process.env.FORCE_COLOR = "0"; diff --git a/src/secrets/apply.test.ts b/src/secrets/apply.test.ts index 1ce50d320..f497f1ef1 100644 --- a/src/secrets/apply.test.ts +++ b/src/secrets/apply.test.ts @@ -146,4 +146,34 @@ describe("secrets apply", () => { expect(nextEnv).not.toContain("sk-openai-plaintext"); expect(nextEnv).toContain("UNRELATED=value"); }); + + it("is idempotent on repeated write applies", async () => { + const plan: SecretsApplyPlan = { + version: 1, + protocolVersion: 1, + generatedAt: new Date().toISOString(), + generatedBy: "manual", + targets: [ + { + type: "models.providers.apiKey", + path: "models.providers.openai.apiKey", + providerId: "openai", + ref: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + }, + ], + options: { + scrubEnv: true, + scrubAuthProfilesForProviderTargets: true, + scrubLegacyAuthJson: true, + }, + }; + + const first = await runSecretsApply({ plan, env, write: true }); + expect(first.changed).toBe(true); + + const second = await runSecretsApply({ plan, env, write: true }); + expect(second.mode).toBe("write"); + expect(second.changed).toBe(false); + expect(second.changedFiles).toEqual([]); + }); }); diff --git a/src/secrets/apply.ts b/src/secrets/apply.ts index e57506d1b..9ef60687c 100644 --- a/src/secrets/apply.ts +++ b/src/secrets/apply.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { isDeepStrictEqual } from "node:util"; import { listAgentIds, resolveAgentDir } from "../agents/agent-scope.js"; import { loadAuthProfileStoreForSecretsRuntime } from "../agents/auth-profiles.js"; import { resolveAuthStorePath } from "../agents/auth-profiles/paths.js"; @@ -62,36 +63,49 @@ function getByDotPath(root: unknown, pathLabel: string): unknown { return cursor; } -function setByDotPath(root: OpenClawConfig, pathLabel: string, value: unknown): void { +function setByDotPath(root: OpenClawConfig, pathLabel: string, value: unknown): boolean { const segments = parseDotPath(pathLabel); if (segments.length === 0) { throw new Error("Target path is empty."); } let cursor: Record = root as unknown as Record; + let changed = false; for (const segment of segments.slice(0, -1)) { const existing = cursor[segment]; if (!isRecord(existing)) { cursor[segment] = {}; + changed = true; } cursor = cursor[segment] as Record; } - cursor[segments[segments.length - 1]] = value; + const leaf = segments[segments.length - 1] ?? ""; + const previous = cursor[leaf]; + if (!isDeepStrictEqual(previous, value)) { + cursor[leaf] = value; + changed = true; + } + return changed; } -function deleteByDotPath(root: OpenClawConfig, pathLabel: string): void { +function deleteByDotPath(root: OpenClawConfig, pathLabel: string): boolean { const segments = parseDotPath(pathLabel); if (segments.length === 0) { - return; + return false; } let cursor: Record = root as unknown as Record; for (const segment of segments.slice(0, -1)) { const existing = cursor[segment]; if (!isRecord(existing)) { - return; + return false; } cursor = existing; } - delete cursor[segments[segments.length - 1]]; + const leaf = segments[segments.length - 1] ?? ""; + if (!Object.prototype.hasOwnProperty.call(cursor, leaf)) { + return false; + } + delete cursor[leaf]; + return true; } function parseEnvValue(raw: string): string { @@ -219,9 +233,11 @@ async function projectPlanState(params: { scrubbedValues.add(previous.trim()); } const refPath = resolveGoogleChatRefPath(target.path); - setByDotPath(nextConfig, refPath, target.ref); - deleteByDotPath(nextConfig, target.path); - changedFiles.add(resolveUserPath(snapshot.path)); + const wroteRef = setByDotPath(nextConfig, refPath, target.ref); + const deletedLegacy = deleteByDotPath(nextConfig, target.path); + if (wroteRef || deletedLegacy) { + changedFiles.add(resolveUserPath(snapshot.path)); + } continue; } @@ -229,8 +245,10 @@ async function projectPlanState(params: { if (isNonEmptyString(previous)) { scrubbedValues.add(previous.trim()); } - setByDotPath(nextConfig, target.path, target.ref); - changedFiles.add(resolveUserPath(snapshot.path)); + const wroteRef = setByDotPath(nextConfig, target.path, target.ref); + if (wroteRef) { + changedFiles.add(resolveUserPath(snapshot.path)); + } if (target.type === "models.providers.apiKey" && target.providerId) { providerTargets.add(normalizeProviderId(target.providerId)); } diff --git a/src/secrets/audit.test.ts b/src/secrets/audit.test.ts index 1d6681a79..ecfbfa2a7 100644 --- a/src/secrets/audit.test.ts +++ b/src/secrets/audit.test.ts @@ -9,6 +9,7 @@ describe("secrets audit", () => { let stateDir = ""; let configPath = ""; let authStorePath = ""; + let authJsonPath = ""; let envPath = ""; let env: NodeJS.ProcessEnv; @@ -17,6 +18,7 @@ describe("secrets audit", () => { stateDir = path.join(rootDir, ".openclaw"); configPath = path.join(stateDir, "openclaw.json"); authStorePath = path.join(stateDir, "agents", "main", "agent", "auth-profiles.json"); + authJsonPath = path.join(stateDir, "agents", "main", "agent", "auth.json"); envPath = path.join(stateDir, ".env"); env = { ...process.env, @@ -80,4 +82,27 @@ describe("secrets audit", () => { expect(report.findings.some((entry) => entry.code === "REF_SHADOWED")).toBe(true); expect(report.findings.some((entry) => entry.code === "PLAINTEXT_FOUND")).toBe(true); }); + + it("does not mutate legacy auth.json during audit", async () => { + await fs.rm(authStorePath, { force: true }); + await fs.writeFile( + authJsonPath, + `${JSON.stringify( + { + openai: { + type: "api_key", + key: "sk-legacy-auth-json", + }, + }, + null, + 2, + )}\n`, + "utf8", + ); + + const report = await runSecretsAudit({ env }); + expect(report.findings.some((entry) => entry.code === "LEGACY_RESIDUE")).toBe(true); + await expect(fs.stat(authJsonPath)).resolves.toBeTruthy(); + await expect(fs.stat(authStorePath)).rejects.toMatchObject({ code: "ENOENT" }); + }); }); diff --git a/src/secrets/audit.ts b/src/secrets/audit.ts index 606035208..e73b24bde 100644 --- a/src/secrets/audit.ts +++ b/src/secrets/audit.ts @@ -510,76 +510,86 @@ export async function runSecretsAudit( } = {}, ): Promise { const env = params.env ?? process.env; - const io = createSecretsConfigIO({ env }); - const { snapshot } = await io.readConfigFileSnapshotForWrite(); - const configPath = resolveUserPath(snapshot.path); - const defaults = snapshot.valid ? snapshot.config.secrets?.defaults : undefined; + const previousAuthStoreReadOnly = process.env.OPENCLAW_AUTH_STORE_READONLY; + process.env.OPENCLAW_AUTH_STORE_READONLY = "1"; + try { + const io = createSecretsConfigIO({ env }); + const snapshot = await io.readConfigFileSnapshot(); + const configPath = resolveUserPath(snapshot.path); + const defaults = snapshot.valid ? snapshot.config.secrets?.defaults : undefined; - const collector: AuditCollector = { - findings: [], - refAssignments: [], - configProviderRefPaths: new Map(), - authProviderState: new Map(), - filesScanned: new Set([configPath]), - }; + const collector: AuditCollector = { + findings: [], + refAssignments: [], + configProviderRefPaths: new Map(), + authProviderState: new Map(), + filesScanned: new Set([configPath]), + }; - const stateDir = resolveStateDir(env, os.homedir); - const envPath = path.join(resolveConfigDir(env, os.homedir), ".env"); - const config = snapshot.valid ? snapshot.config : ({} as OpenClawConfig); + const stateDir = resolveStateDir(env, os.homedir); + const envPath = path.join(resolveConfigDir(env, os.homedir), ".env"); + const config = snapshot.valid ? snapshot.config : ({} as OpenClawConfig); - if (snapshot.valid) { - collectConfigSecrets({ - config, - configPath, - collector, - }); - for (const authStorePath of collectAuthStorePaths(config, stateDir)) { - collectAuthStoreSecrets({ - authStorePath, + if (snapshot.valid) { + collectConfigSecrets({ + config, + configPath, collector, - defaults, + }); + for (const authStorePath of collectAuthStorePaths(config, stateDir)) { + collectAuthStoreSecrets({ + authStorePath, + collector, + defaults, + }); + } + await collectUnresolvedRefFindings({ + collector, + config, + env, + }); + collectShadowingFindings(collector); + } else { + addFinding(collector, { + code: "REF_UNRESOLVED", + severity: "error", + file: configPath, + jsonPath: "", + message: "Config is invalid; cannot validate secret references reliably.", }); } - await collectUnresolvedRefFindings({ + + collectEnvPlaintext({ + envPath, collector, - config, - env, }); - collectShadowingFindings(collector); - } else { - addFinding(collector, { - code: "REF_UNRESOLVED", - severity: "error", - file: configPath, - jsonPath: "", - message: "Config is invalid; cannot validate secret references reliably.", + collectAuthJsonResidue({ + stateDir, + collector, }); + + const summary = summarizeFindings(collector.findings); + const status: SecretsAuditStatus = + summary.unresolvedRefCount > 0 + ? "unresolved" + : collector.findings.length > 0 + ? "findings" + : "clean"; + + return { + version: 1, + status, + filesScanned: [...collector.filesScanned].toSorted(), + summary, + findings: collector.findings, + }; + } finally { + if (previousAuthStoreReadOnly === undefined) { + delete process.env.OPENCLAW_AUTH_STORE_READONLY; + } else { + process.env.OPENCLAW_AUTH_STORE_READONLY = previousAuthStoreReadOnly; + } } - - collectEnvPlaintext({ - envPath, - collector, - }); - collectAuthJsonResidue({ - stateDir, - collector, - }); - - const summary = summarizeFindings(collector.findings); - const status: SecretsAuditStatus = - summary.unresolvedRefCount > 0 - ? "unresolved" - : collector.findings.length > 0 - ? "findings" - : "clean"; - - return { - version: 1, - status, - filesScanned: [...collector.filesScanned].toSorted(), - summary, - findings: collector.findings, - }; } export function resolveSecretsAuditExitCode(report: SecretsAuditReport, check: boolean): number {