From 8e33ebe4713cdd4417cba8bd7533ea11066d2bef Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Tue, 24 Feb 2026 13:19:02 -0600 Subject: [PATCH] Secrets: make runtime activation auth loads read-only --- src/agents/auth-profiles.ts | 1 + src/agents/auth-profiles/store.ts | 27 ++++++-- src/gateway/server.reload.test.ts | 100 ++++++++++++++++++++++++++++++ src/secrets/runtime.test.ts | 48 ++++++++++++++ src/secrets/runtime.ts | 18 ++---- src/secrets/shared.ts | 14 +++++ src/secrets/sops.ts | 6 +- 7 files changed, 191 insertions(+), 23 deletions(-) create mode 100644 src/secrets/shared.ts diff --git a/src/agents/auth-profiles.ts b/src/agents/auth-profiles.ts index d4d1e677e..7bf01847e 100644 --- a/src/agents/auth-profiles.ts +++ b/src/agents/auth-profiles.ts @@ -19,6 +19,7 @@ export { export { clearRuntimeAuthProfileStoreSnapshots, ensureAuthProfileStore, + loadAuthProfileStoreForSecretsRuntime, loadAuthProfileStoreForRuntime, replaceRuntimeAuthProfileStoreSnapshots, loadAuthProfileStore, diff --git a/src/agents/auth-profiles/store.ts b/src/agents/auth-profiles/store.ts index ac94967ba..7e39cc022 100644 --- a/src/agents/auth-profiles/store.ts +++ b/src/agents/auth-profiles/store.ts @@ -11,6 +11,10 @@ import type { AuthProfileCredential, AuthProfileStore, ProfileUsageStats } from type LegacyAuthStore = Record; type CredentialRejectReason = "non_object" | "invalid_type" | "missing_provider"; type RejectedCredentialEntry = { key: string; reason: CredentialRejectReason }; +type LoadAuthProfileStoreOptions = { + allowKeychainPrompt?: boolean; + readOnly?: boolean; +}; const AUTH_PROFILE_TYPES = new Set(["api_key", "oauth", "token"]); @@ -373,16 +377,25 @@ export function loadAuthProfileStore(): AuthProfileStore { function loadAuthProfileStoreForAgent( agentDir?: string, - _options?: { allowKeychainPrompt?: boolean }, + options?: LoadAuthProfileStoreOptions, ): AuthProfileStore { + const readOnly = options?.readOnly === true; const authPath = resolveAuthStorePath(agentDir); const asStore = loadCoercedStoreWithExternalSync(authPath); if (asStore) { + // Runtime secret activation must remain read-only. + if (!readOnly) { + // Sync from external CLI tools on every load + const synced = syncExternalCliCredentials(asStore); + if (synced) { + saveJsonFile(authPath, asStore); + } + } return asStore; } // Fallback: inherit auth-profiles from main agent if subagent has none - if (agentDir) { + if (agentDir && !readOnly) { const mainAuthPath = resolveAuthStorePath(); // without agentDir = main const mainRaw = loadJsonFile(mainAuthPath); const mainStore = coerceAuthStore(mainRaw); @@ -405,8 +418,8 @@ function loadAuthProfileStoreForAgent( } const mergedOAuth = mergeOAuthFileIntoStore(store); - const syncedCli = syncExternalCliCredentials(store); - const shouldWrite = legacy !== null || mergedOAuth || syncedCli; + const syncedCli = readOnly ? false : syncExternalCliCredentials(store); + const shouldWrite = !readOnly && (legacy !== null || mergedOAuth || syncedCli); if (shouldWrite) { saveJsonFile(authPath, store); } @@ -433,7 +446,7 @@ function loadAuthProfileStoreForAgent( export function loadAuthProfileStoreForRuntime( agentDir?: string, - options?: { allowKeychainPrompt?: boolean }, + options?: LoadAuthProfileStoreOptions, ): AuthProfileStore { const store = loadAuthProfileStoreForAgent(agentDir, options); const authPath = resolveAuthStorePath(agentDir); @@ -446,6 +459,10 @@ export function loadAuthProfileStoreForRuntime( return mergeAuthProfileStores(mainStore, store); } +export function loadAuthProfileStoreForSecretsRuntime(agentDir?: string): AuthProfileStore { + return loadAuthProfileStoreForRuntime(agentDir, { readOnly: true, allowKeychainPrompt: false }); +} + export function ensureAuthProfileStore( agentDir?: string, options?: { allowKeychainPrompt?: boolean }, diff --git a/src/gateway/server.reload.test.ts b/src/gateway/server.reload.test.ts index bbed94d77..663e84bc4 100644 --- a/src/gateway/server.reload.test.ts +++ b/src/gateway/server.reload.test.ts @@ -1,4 +1,7 @@ +import fs from "node:fs/promises"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveMainSessionKeyFromConfig } from "../config/sessions.js"; +import { drainSystemEvents } from "../infra/system-events.js"; import { connectOk, installGatewayTestHooks, @@ -170,11 +173,13 @@ describe("gateway hot reload", () => { let prevSkipChannels: string | undefined; let prevSkipGmail: string | undefined; let prevSkipProviders: string | undefined; + let prevOpenAiApiKey: string | undefined; beforeEach(() => { prevSkipChannels = process.env.OPENCLAW_SKIP_CHANNELS; prevSkipGmail = process.env.OPENCLAW_SKIP_GMAIL_WATCHER; prevSkipProviders = process.env.OPENCLAW_SKIP_PROVIDERS; + prevOpenAiApiKey = process.env.OPENAI_API_KEY; process.env.OPENCLAW_SKIP_CHANNELS = "0"; delete process.env.OPENCLAW_SKIP_GMAIL_WATCHER; delete process.env.OPENCLAW_SKIP_PROVIDERS; @@ -196,8 +201,39 @@ describe("gateway hot reload", () => { } else { process.env.OPENCLAW_SKIP_PROVIDERS = prevSkipProviders; } + if (prevOpenAiApiKey === undefined) { + delete process.env.OPENAI_API_KEY; + } else { + process.env.OPENAI_API_KEY = prevOpenAiApiKey; + } }); + async function writeEnvRefConfig() { + const configPath = process.env.OPENCLAW_CONFIG_PATH; + if (!configPath) { + throw new Error("OPENCLAW_CONFIG_PATH is not set"); + } + await fs.writeFile( + configPath, + `${JSON.stringify( + { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "env", id: "OPENAI_API_KEY" }, + models: [], + }, + }, + }, + }, + null, + 2, + )}\n`, + "utf8", + ); + } + it("applies hot reload actions and emits restart signal", async () => { await withGatewayServer(async () => { const onHotReload = hoisted.getOnHotReload(); @@ -302,6 +338,70 @@ describe("gateway hot reload", () => { expect(signalSpy).toHaveBeenCalledTimes(1); }); }); + + it("fails startup when required secret refs are unresolved", async () => { + await writeEnvRefConfig(); + delete process.env.OPENAI_API_KEY; + await expect(withGatewayServer(async () => {})).rejects.toThrow( + "Startup failed: required secrets are unavailable", + ); + }); + + it("emits one-shot degraded and recovered system events during secret reload transitions", async () => { + await writeEnvRefConfig(); + process.env.OPENAI_API_KEY = "sk-startup"; + + await withGatewayServer(async () => { + const onHotReload = hoisted.getOnHotReload(); + expect(onHotReload).toBeTypeOf("function"); + const sessionKey = resolveMainSessionKeyFromConfig(); + const plan = { + changedPaths: ["models.providers.openai.apiKey"], + restartGateway: false, + restartReasons: [], + hotReasons: ["models.providers.openai.apiKey"], + reloadHooks: false, + restartGmailWatcher: false, + restartBrowserControl: false, + restartCron: false, + restartHeartbeat: false, + restartChannels: new Set(), + noopPaths: [], + }; + const nextConfig = { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "env", id: "OPENAI_API_KEY" }, + models: [], + }, + }, + }, + }; + + delete process.env.OPENAI_API_KEY; + await expect(onHotReload?.(plan, nextConfig)).rejects.toThrow( + 'Environment variable "OPENAI_API_KEY" is missing or empty.', + ); + const degradedEvents = drainSystemEvents(sessionKey); + expect(degradedEvents.some((event) => event.includes("[SECRETS_RELOADER_DEGRADED]"))).toBe( + true, + ); + + await expect(onHotReload?.(plan, nextConfig)).rejects.toThrow( + 'Environment variable "OPENAI_API_KEY" is missing or empty.', + ); + expect(drainSystemEvents(sessionKey)).toEqual([]); + + process.env.OPENAI_API_KEY = "sk-recovered"; + await expect(onHotReload?.(plan, nextConfig)).resolves.toBeUndefined(); + const recoveredEvents = drainSystemEvents(sessionKey); + expect(recoveredEvents.some((event) => event.includes("[SECRETS_RELOADER_RECOVERED]"))).toBe( + true, + ); + }); + }); }); describe("gateway agents", () => { diff --git a/src/secrets/runtime.test.ts b/src/secrets/runtime.test.ts index 5920ba0d2..396b01d00 100644 --- a/src/secrets/runtime.test.ts +++ b/src/secrets/runtime.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { ensureAuthProfileStore } from "../agents/auth-profiles.js"; import { loadConfig, type OpenClawConfig } from "../config/config.js"; @@ -156,4 +159,49 @@ describe("secrets runtime snapshot", () => { key: "sk-runtime", }); }); + + it("does not write inherited auth stores during runtime secret activation", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-secrets-runtime-")); + const stateDir = path.join(root, ".openclaw"); + const mainAgentDir = path.join(stateDir, "agents", "main", "agent"); + const workerStorePath = path.join(stateDir, "agents", "worker", "agent", "auth-profiles.json"); + const prevStateDir = process.env.OPENCLAW_STATE_DIR; + + try { + await fs.mkdir(mainAgentDir, { recursive: true }); + await fs.writeFile( + path.join(mainAgentDir, "auth-profiles.json"), + JSON.stringify({ + version: 1, + profiles: { + "openai:default": { + type: "api_key", + provider: "openai", + keyRef: { source: "env", id: "OPENAI_API_KEY" }, + }, + }, + }), + "utf8", + ); + process.env.OPENCLAW_STATE_DIR = stateDir; + + await prepareSecretsRuntimeSnapshot({ + config: { + agents: { + list: [{ id: "worker" }], + }, + }, + env: { OPENAI_API_KEY: "sk-runtime-worker" }, + }); + + await expect(fs.access(workerStorePath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + if (prevStateDir === undefined) { + delete process.env.OPENCLAW_STATE_DIR; + } else { + process.env.OPENCLAW_STATE_DIR = prevStateDir; + } + await fs.rm(root, { recursive: true, force: true }); + } + }); }); diff --git a/src/secrets/runtime.ts b/src/secrets/runtime.ts index cbd8fb05e..ee63a696f 100644 --- a/src/secrets/runtime.ts +++ b/src/secrets/runtime.ts @@ -3,7 +3,7 @@ import { listAgentIds, resolveAgentDir } from "../agents/agent-scope.js"; import type { AuthProfileCredential, AuthProfileStore } from "../agents/auth-profiles.js"; import { clearRuntimeAuthProfileStoreSnapshots, - loadAuthProfileStoreForRuntime, + loadAuthProfileStoreForSecretsRuntime, replaceRuntimeAuthProfileStoreSnapshots, } from "../agents/auth-profiles.js"; import { @@ -14,6 +14,7 @@ import { import { isSecretRef, type SecretRef } from "../config/types.secrets.js"; import { resolveUserPath } from "../utils.js"; import { readJsonPointer } from "./json-pointer.js"; +import { isNonEmptyString, isRecord, normalizePositiveInt } from "./shared.js"; import { decryptSopsJsonFile, DEFAULT_SOPS_TIMEOUT_MS } from "./sops.js"; type SecretResolverWarningCode = "SECRETS_REF_OVERRIDES_PLAINTEXT"; @@ -73,14 +74,6 @@ function cloneSnapshot(snapshot: PreparedSecretsRuntimeSnapshot): PreparedSecret }; } -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - -function isNonEmptyString(value: unknown): value is string { - return typeof value === "string" && value.trim().length > 0; -} - async function decryptSopsFile(config: OpenClawConfig): Promise { const fileSource = config.secrets?.sources?.file; if (!fileSource) { @@ -93,10 +86,7 @@ async function decryptSopsFile(config: OpenClawConfig): Promise { } const resolvedPath = resolveUserPath(fileSource.path); - const timeoutMs = - typeof fileSource.timeoutMs === "number" && Number.isFinite(fileSource.timeoutMs) - ? Math.max(1, Math.floor(fileSource.timeoutMs)) - : DEFAULT_SOPS_TIMEOUT_MS; + const timeoutMs = normalizePositiveInt(fileSource.timeoutMs, DEFAULT_SOPS_TIMEOUT_MS); return await decryptSopsJsonFile({ path: resolvedPath, timeoutMs, @@ -275,7 +265,7 @@ export async function prepareSecretsRuntimeSnapshot(params: { warnings, }); - const loadAuthStore = params.loadAuthStore ?? loadAuthProfileStoreForRuntime; + const loadAuthStore = params.loadAuthStore ?? loadAuthProfileStoreForSecretsRuntime; const candidateDirs = params.agentDirs?.length ? [...new Set(params.agentDirs.map((entry) => resolveUserPath(entry)))] : collectCandidateAgentDirs(resolvedConfig); diff --git a/src/secrets/shared.ts b/src/secrets/shared.ts new file mode 100644 index 000000000..e0c293f14 --- /dev/null +++ b/src/secrets/shared.ts @@ -0,0 +1,14 @@ +export function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +export function isNonEmptyString(value: unknown): value is string { + return typeof value === "string" && value.trim().length > 0; +} + +export function normalizePositiveInt(value: unknown, fallback: number): number { + if (typeof value === "number" && Number.isFinite(value)) { + return Math.max(1, Math.floor(value)); + } + return Math.max(1, Math.floor(fallback)); +} diff --git a/src/secrets/sops.ts b/src/secrets/sops.ts index 81dae502b..3437f6812 100644 --- a/src/secrets/sops.ts +++ b/src/secrets/sops.ts @@ -2,15 +2,13 @@ import crypto from "node:crypto"; import fs from "node:fs"; import path from "node:path"; import { runExec } from "../process/exec.js"; +import { normalizePositiveInt } from "./shared.js"; export const DEFAULT_SOPS_TIMEOUT_MS = 5_000; const MAX_SOPS_OUTPUT_BYTES = 10 * 1024 * 1024; function normalizeTimeoutMs(value: number | undefined): number { - if (typeof value === "number" && Number.isFinite(value)) { - return Math.max(1, Math.floor(value)); - } - return DEFAULT_SOPS_TIMEOUT_MS; + return normalizePositiveInt(value, DEFAULT_SOPS_TIMEOUT_MS); } function isTimeoutError(message: string | undefined): boolean {