From 7d008a8db22608be24c8ad1544de97a66d45da95 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 16:09:33 -0700 Subject: [PATCH] fix(auth): honor shared-store ownership in temp-state exec and agent deletion (#130437) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from #130264: agent exec temporary state now preserves the original shared auth root, so temp-state runs read-through portable static profiles (local precedence and cooldowns preserved, no inherited-secret copies); and agent deletion safety consults resolveSharedAuthStoreOwnership, so the implicit legacy main owner is protected only while the legacy store owns credentials — post-relocation main is deletable and shared credentials provably survive. Explicitly bound inheritance owners stay protected. --- docs/auth-credential-semantics.md | 2 + src/agents/agent-delete-safety.ts | 14 +- .../auth-profiles/legacy-source-diagnostic.ts | 8 +- src/agents/auth-profiles/store.ts | 154 +++++--- src/commands/agent-exec.auth.test.ts | 363 ++++++++++++++++++ src/commands/agent-exec.test.ts | 196 ---------- src/commands/agent-exec.ts | 9 +- src/commands/agents.commands.delete.ts | 8 +- src/commands/agents.delete.test.ts | 26 +- .../server-methods/agents-mutate.test.ts | 40 +- src/gateway/server-methods/agents.ts | 7 +- 11 files changed, 551 insertions(+), 276 deletions(-) create mode 100644 src/commands/agent-exec.auth.test.ts diff --git a/docs/auth-credential-semantics.md b/docs/auth-credential-semantics.md index 102cb6197b5c..4b8779fce8f2 100644 --- a/docs/auth-credential-semantics.md +++ b/docs/auth-credential-semantics.md @@ -59,6 +59,8 @@ Explicit copy flows, such as `openclaw agents add`, use this portability policy: Non-portable profiles remain available through the shared read-through base unless the target agent signs in separately and creates its own local profile. +`openclaw agent exec` preserves the original shared-store root when switching to temporary run state. Its bounded credential scope reads portable `api_key` and `token` profiles from that shared store without persisting copies; the configured agent's local profiles still win. Shared OAuth profiles are excluded from this temporary scope, even with `copyToAgents: true`, so the run does not acquire another refresh owner. `--auth-env-only` disables stored credential access entirely. + ## Config-only auth routes `auth.profiles` entries with `mode: "aws-sdk"` are routing metadata, not stored credentials. They are valid when the target provider uses `models.providers..auth: "aws-sdk"`, the route the plugin-owned Amazon Bedrock setup writes. These profile ids may appear in `auth.order` and session overrides even when no matching entry exists in the credential store. diff --git a/src/agents/agent-delete-safety.ts b/src/agents/agent-delete-safety.ts index c873978d2715..b003110ffa36 100644 --- a/src/agents/agent-delete-safety.ts +++ b/src/agents/agent-delete-safety.ts @@ -4,7 +4,11 @@ import { isPathInside } from "../infra/path-guards.js"; import { normalizeAgentId } from "../routing/session-key.js"; import { isSameOpenClawAgentDatabasePath } from "../state/openclaw-agent-db-registry.js"; import { listAgentEntries, resolveAgentWorkspaceDir } from "./agent-scope.js"; -import type { SharedAuthStoreOwnership } from "./auth-profiles/path-resolve.js"; +import { + resolveSharedAuthStoreOwnership, + type SharedAuthStoreOwnership, +} from "./auth-profiles/path-resolve.js"; +import { resolveLegacyInheritedAuthAgentId } from "./legacy-inherited-auth-dir.js"; import { resolveCanonicalWorkspacePath } from "./workspace-state-identity.js"; /** True when deleting this agent database would remove the legacy shared auth store. */ @@ -23,6 +27,14 @@ export function formatSharedAuthStoreOwnerDeleteError(agentId: string): string { return `Agent "${agentId}" owns the legacy shared auth store and cannot be deleted. Run openclaw doctor --fix to migrate shared auth, then retry.`; } +export function isInheritedAuthStoreOwner(cfg: OpenClawConfig, agentId: string): boolean { + // Relocation retires the implicit agent owner, but explicit bindings must be re-pointed. + const explicitOwner = cfg.agents?.defaults?.authInheritance?.agentId?.trim(); + if (!explicitOwner && resolveSharedAuthStoreOwnership().location !== "legacy-main") { + return false; + } + return agentId === normalizeAgentId(resolveLegacyInheritedAuthAgentId(cfg)); +} function workspacePathsOverlap(left: string, right: string): boolean { const normalizedLeft = resolveCanonicalWorkspacePath(left.replaceAll("\0", "")); const normalizedRight = resolveCanonicalWorkspacePath(right.replaceAll("\0", "")); diff --git a/src/agents/auth-profiles/legacy-source-diagnostic.ts b/src/agents/auth-profiles/legacy-source-diagnostic.ts index 57f6fd1c8154..7d41bcce8e73 100644 --- a/src/agents/auth-profiles/legacy-source-diagnostic.ts +++ b/src/agents/auth-profiles/legacy-source-diagnostic.ts @@ -24,8 +24,8 @@ function isCredentialSource(source: LegacyAuthProfileSource): boolean { return source.kind !== "auth-state"; } -function resolveAuthProfileOwnerPath(agentDir?: string): string { - return agentDir ? resolveAuthProfileDatabasePath(agentDir) : resolveSharedAuthStorePath(); +function resolveAuthProfileOwnerPath(agentDir?: string, env?: NodeJS.ProcessEnv): string { + return agentDir ? resolveAuthProfileDatabasePath(agentDir) : resolveSharedAuthStorePath(env); } export function hasLegacyAuthProfileCredentialSource(agentDir?: string): boolean { @@ -121,9 +121,9 @@ export class AuthProfileStoreUnreadableError extends Error { readonly code = "AUTH_PROFILE_STORE_UNREADABLE" as const; readonly action = AUTH_PROFILE_MIGRATION_COMMAND; - constructor(agentDir?: string) { + constructor(agentDir?: string, env?: NodeJS.ProcessEnv) { super( - `Auth profile store ${shortenHomePath(resolveAuthProfileOwnerPath(agentDir))} is unreadable; run ${AUTH_PROFILE_MIGRATION_COMMAND}.`, + `Auth profile store ${shortenHomePath(resolveAuthProfileOwnerPath(agentDir, env))} is unreadable; run ${AUTH_PROFILE_MIGRATION_COMMAND}.`, ); this.name = "AuthProfileStoreUnreadableError"; } diff --git a/src/agents/auth-profiles/store.ts b/src/agents/auth-profiles/store.ts index 6eeb8323648d..e961b3eea667 100644 --- a/src/agents/auth-profiles/store.ts +++ b/src/agents/auth-profiles/store.ts @@ -36,12 +36,17 @@ import { shouldUseMainOwnerForLocalOAuthCredential, type PersistedAuthProfileStores, } from "./ownership.js"; -import { resolveSharedAuthStorePath as resolveSharedAuthPath } from "./path-resolve.js"; +import { + resolveSharedAuthStoreOwnership, + resolveSharedAuthStorePath as resolveSharedAuthPath, +} from "./path-resolve.js"; import { buildPersistedAuthProfileSecretsStore, loadPersistedAuthProfileStore, + loadPersistedSharedAuthProfileStore, mergeAuthProfileStores, } from "./persisted.js"; +import { resolveAuthProfilePortability } from "./portability.js"; import { getRuntimeExternalCliProfileIds, mergeRuntimeExternalProfileReferences, @@ -64,6 +69,7 @@ import { deferAuthProfilePostCommitPublication, deletePersistedAuthProfileStoreRaw, inspectPersistedAuthProfileStoreRaw, + inspectPersistedSharedAuthProfileStoreRaw, readPersistedAuthProfileStoreRaw, readPersistedAuthProfileStateRaw, resolveAuthProfileDatabasePath as resolveAgentAuthPath, @@ -97,7 +103,9 @@ type SaveAuthProfileStoreOptions = { }; const INLINE_OAUTH_TOKEN_FIELDS = ["access", "refresh", "idToken"] as const; -type AuthProfileRuntimeMode = { kind: "env-only" } | { kind: "agent-dir"; agentDir: string }; +type AuthProfileRuntimeMode = + | { kind: "env-only" } + | { kind: "agent-dir"; agentDir: string; sharedStore?: AuthProfileStore }; const authProfileRuntimeMode = new AsyncLocalStorage(); @@ -111,8 +119,50 @@ export function withEnvOnlyAuthProfileStore(run: () => T): T { } /** Run a bounded operation against one existing persisted auth store. */ -export function withAuthProfileStoreAgentDir(agentDir: string, run: () => T): T { - return authProfileRuntimeMode.run({ kind: "agent-dir", agentDir }, run); +export function withAuthProfileStoreAgentDir( + agentDir: string, + sharedStateDir: string, + run: () => T, +): T { + const env = { ...process.env, OPENCLAW_STATE_DIR: sharedStateDir }; + let sharedStore: AuthProfileStore | undefined; + if (resolveSharedAuthStoreOwnership(env).location === "state-db") { + const shared = loadPersistedSharedAuthProfileStore(env); + if (!shared && inspectPersistedSharedAuthProfileStoreRaw(env).status !== "missing") { + throw new AuthProfileStoreUnreadableError(undefined, env); + } + sharedStore = shared ?? createEmptyAuthProfileStore(); + } + // Temporary runs must not acquire a second OAuth refresh owner. Keep this + // read-through view in the operation scope, never in a persisted agent store. + if (sharedStore) { + sharedStore.profiles = Object.fromEntries( + Object.entries(sharedStore.profiles).filter( + ([, credential]) => + resolveAuthProfilePortability(credential).reason === "portable-static-credential", + ), + ); + pruneAuthProfileStoreReferences(sharedStore, new Set(Object.keys(sharedStore.profiles))); + } + return authProfileRuntimeMode.run({ kind: "agent-dir", agentDir, sharedStore }, run); +} + +function getScopedSharedAuthStore(): AuthProfileStore | undefined { + const mode = authProfileRuntimeMode.getStore(); + return mode?.kind === "agent-dir" ? mode.sharedStore : undefined; +} + +function applyScopedAuthReadThrough(store: AuthProfileStore): AuthProfileStore { + const shared = getScopedSharedAuthStore(); + if (!shared) { + return store; + } + const merged = mergeAuthProfileStores(cloneAuthProfileStore(shared), store); + return setRuntimeLocalProfileMetadata( + merged, + Object.keys(store.profiles), + runtimeStoreInheritsMainState(merged, store), + ); } function isEnvOnlyAuthProfileRuntime(): boolean { @@ -286,6 +336,11 @@ function resolveRuntimeAuthProfileStore( agentDir?: string, options?: Pick, ): AuthProfileStore | null { + // Ambient snapshots may include non-portable shared profiles. A bounded exec + // scope composes its view from the actual local store and its filtered base. + if (getScopedSharedAuthStore()) { + return null; + } const mainKey = options?.inheritedAuthDir ? resolveAgentAuthPath(options.inheritedAuthDir) : resolveSharedAuthPath(); @@ -471,6 +526,18 @@ function shouldKeepProfileInLocalStore(params: { persistedStores: PersistedAuthProfileStores; externalProfiles: () => RuntimeExternalOAuthProfile[]; }): boolean { + const inherited = getScopedSharedAuthStore()?.profiles[params.profileId]; + if (inherited && !params.persistedStores.localStore?.profiles[params.profileId]) { + // Runtime state updates must not turn read-through credentials into local copies. + // Compare persisted shapes so a materialized SecretRef stays inherited too. + const secrets = buildPersistedAuthProfileSecretsStore({ + version: AUTH_STORE_VERSION, + profiles: { [params.profileId]: params.credential }, + }); + if (isDeepStrictEqual(secrets.profiles[params.profileId], inherited)) { + return false; + } + } if (params.credential.type !== "oauth") { return true; } @@ -715,9 +782,12 @@ function runtimeStoreInheritsMainState( } function listRuntimeLocalProfileIds( - store: AuthProfileStore, + store: RuntimeAuthProfileStore, mainStore?: AuthProfileStore, ): string[] { + if (store.runtimeLocalProfileIds) { + return store.runtimeLocalProfileIds; + } return Object.entries(store.profiles).flatMap(([profileId, credential]) => mainStore && shouldUseMainOwnerForLocalOAuthCredential({ @@ -917,13 +987,11 @@ export function loadAuthProfileStore(): AuthProfileStore { return createEmptyAuthProfileStore(); } const agentDir = resolveRuntimeAuthProfileAgentDir(); - const asStore = loadPersistedAuthProfileStore(agentDir); - if (asStore) { - return overlayExternalAuthProfiles(markRuntimePersistedProfiles(asStore), { agentDir }); - } - - const store: AuthProfileStore = { version: AUTH_STORE_VERSION, profiles: {} }; - return overlayExternalAuthProfiles(markRuntimePersistedProfiles(store), { agentDir }); + const store = loadPersistedAuthProfileStore(agentDir) ?? createEmptyAuthProfileStore(); + return overlayExternalAuthProfiles( + applyScopedAuthReadThrough(markRuntimePersistedProfiles(store)), + { agentDir }, + ); } function loadAuthProfileStoreForAgent( @@ -936,64 +1004,39 @@ function loadAuthProfileStoreForAgent( const effectiveAgentDir = resolveRuntimeAuthProfileAgentDir(agentDir); const effectiveOptions = resolveRuntimeAuthProfileLoadOptions(options); assertAuthProfileMigrationReady(effectiveAgentDir); - const asStore = loadPersistedAuthProfileStore( + const store = loadPersistedAuthProfileStore( effectiveAgentDir, resolvePersistedLoadOptions(effectiveOptions), ); - if (asStore) { - const legacySources = listLegacyAuthProfileSources({ agentDir: effectiveAgentDir }); - const credentialSources = legacySources.filter((source) => source.kind !== "auth-state"); - // A populated canonical store already owns this agent's credentials, so a - // retired file beside it is unarchived bytes rather than pending migration. - // Only an empty store means the credentials still live solely in that file. - if (credentialSources.length > 0 && Object.keys(asStore.profiles).length === 0) { - const migrationError = new AuthProfileMigrationRequiredError({ - agentDir: effectiveAgentDir, - sources: credentialSources, - }); - markAuthProfileMigrationRequired(effectiveAgentDir, migrationError); - throw migrationError; - } - warnLegacyAuthProfileSourcesIgnored({ - agentDir: effectiveAgentDir, - sources: legacySources, - }); - clearAuthProfileMigrationRequired(effectiveAgentDir); - const synced = maybeSyncPersistedExternalCliAuthProfiles({ - store: asStore, - agentDir: effectiveAgentDir, - options: effectiveOptions, - }); - return markRuntimePersistedProfiles(synced.store); - } - - const inspection = inspectPersistedAuthProfileStoreRaw( - effectiveAgentDir, - effectiveOptions?.database, - ); - if (inspection.status !== "missing") { + if ( + !store && + inspectPersistedAuthProfileStoreRaw(effectiveAgentDir, effectiveOptions?.database).status !== + "missing" + ) { throw new AuthProfileStoreUnreadableError(effectiveAgentDir); } const legacySources = listLegacyAuthProfileSources({ agentDir: effectiveAgentDir }); const credentialSources = legacySources.filter((source) => source.kind !== "auth-state"); - if (credentialSources.length > 0) { - throw new AuthProfileMigrationRequiredError({ + // A populated canonical store owns credentials; retired files beside it are + // unarchived bytes. An empty or absent store still requires migration. + if (credentialSources.length > 0 && (!store || Object.keys(store.profiles).length === 0)) { + const migrationError = new AuthProfileMigrationRequiredError({ agentDir: effectiveAgentDir, sources: credentialSources, }); + if (store) { + markAuthProfileMigrationRequired(effectiveAgentDir, migrationError); + } + throw migrationError; } warnLegacyAuthProfileSourcesIgnored({ agentDir: effectiveAgentDir, sources: legacySources }); clearAuthProfileMigrationRequired(effectiveAgentDir); - const store: AuthProfileStore = { - version: AUTH_STORE_VERSION, - profiles: {}, - }; const synced = maybeSyncPersistedExternalCliAuthProfiles({ - store, + store: store ?? createEmptyAuthProfileStore(), agentDir: effectiveAgentDir, options: effectiveOptions, }); - return markRuntimePersistedProfiles(synced.store); + return applyScopedAuthReadThrough(markRuntimePersistedProfiles(synced.store)); } /** Loads the effective runtime store for an agent, including inherited main profiles. */ @@ -1129,6 +1172,7 @@ export function ensureAuthProfileStore( ); if (!runtimeStore) { if ( + !getScopedSharedAuthStore() && hasScopedExternalCliOverlay(externalCli) && (store.runtimeExternalProfileIds?.length ?? 0) > 0 ) { @@ -1210,6 +1254,10 @@ export function findPersistedAuthProfileCredential(params: { const agentDir = resolveRuntimeAuthProfileAgentDir(params.agentDir); const requestedStore = loadPersistedAuthProfileStore(agentDir); const requestedProfile = requestedStore?.profiles[params.profileId]; + const scopedSharedStore = getScopedSharedAuthStore(); + if (scopedSharedStore) { + return requestedProfile ?? scopedSharedStore.profiles[params.profileId]; + } if (requestedProfile || !agentDir) { return requestedProfile; } diff --git a/src/commands/agent-exec.auth.test.ts b/src/commands/agent-exec.auth.test.ts new file mode 100644 index 000000000000..23bd6d0b7567 --- /dev/null +++ b/src/commands/agent-exec.auth.test.ts @@ -0,0 +1,363 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; +import { + ensureAuthProfileStore, + findPersistedAuthProfileCredential, + loadAuthProfileStoreForRuntime, + resolveAuthProfileOrder, + resolvePersistedAuthProfileOwnerAgentDir, +} from "../agents/auth-profiles.js"; +import { + clearRuntimeAuthProfileStoreSnapshots, + setRuntimeAuthProfileStoreSnapshot, +} from "../agents/auth-profiles/runtime-snapshots.js"; +import { + inspectPersistedAuthProfileStoreRaw, + readPersistedAuthProfileStoreRaw, + writePersistedAuthProfileStoreRaw, +} from "../agents/auth-profiles/sqlite.js"; +import type { RuntimeEnv } from "../runtime.js"; +import { writeConfigMachineState } from "../state/config-machine-state.js"; +import { withOpenClawTestState } from "../test-utils/openclaw-test-state.js"; +import { agentExecCommand } from "./agent-exec.js"; + +const tempDirs = useAutoCleanupTempDirTracker(afterEach); + +function createRuntime() { + return { runtime: { log: vi.fn(), error: vi.fn(), exit: vi.fn() } satisfies RuntimeEnv }; +} + +function successResult() { + return { + payloads: [{ text: "done" }], + meta: { durationMs: 1, finalAssistantVisibleText: "done" }, + }; +} + +afterEach(() => { + vi.restoreAllMocks(); + clearRuntimeAuthProfileStoreSnapshots(); +}); + +describe("agent exec stored auth", () => { + it("skips external Codex CLI credentials under --auth-env-only", async () => { + const codexHome = tempDirs.make("openclaw-agent-exec-codex-home-"); + await fs.writeFile( + path.join(codexHome, "auth.json"), + JSON.stringify({ + auth_mode: "chatgpt", + tokens: { access_token: "test-access", refresh_token: "test-refresh" }, + }), + "utf8", + ); + const previousCodexHome = process.env.CODEX_HOME; + const previousOpenAiApiKey = process.env.OPENAI_API_KEY; + const previousDatabaseUrl = process.env.DATABASE_URL; + process.env.CODEX_HOME = codexHome; + process.env.OPENAI_API_KEY = "test-openai-key"; + process.env.DATABASE_URL = "postgres://test.invalid/database"; + const { runtime } = createRuntime(); + let profileIds: string[] = []; + let runtimeProfileIds: string[] = []; + let hostExecApiKey: string | undefined; + let hostExecDatabaseUrl: string | undefined; + try { + const { withHostExecInheritedEnvOmitted } = await import("../infra/host-env-security.js"); + await withHostExecInheritedEnvOmitted(["DATABASE_URL"], () => + agentExecCommand("inspect", { authEnvOnly: true }, runtime, { + runAgent: vi.fn(async () => { + profileIds = Object.keys( + ensureAuthProfileStore(undefined, { + allowKeychainPrompt: false, + externalCliProviderIds: ["openai"], + }).profiles, + ); + runtimeProfileIds = Object.keys( + loadAuthProfileStoreForRuntime(undefined, { + allowKeychainPrompt: false, + externalCliProviderIds: ["openai"], + }).profiles, + ); + const { sanitizeHostExecEnv } = await import("../infra/host-env-security.js"); + const hostExecEnv = sanitizeHostExecEnv({ baseEnv: process.env }); + hostExecApiKey = hostExecEnv.OPENAI_API_KEY; + hostExecDatabaseUrl = hostExecEnv.DATABASE_URL; + return successResult(); + }), + }), + ); + } finally { + if (previousCodexHome === undefined) { + delete process.env.CODEX_HOME; + } else { + process.env.CODEX_HOME = previousCodexHome; + } + if (previousOpenAiApiKey === undefined) { + delete process.env.OPENAI_API_KEY; + } else { + process.env.OPENAI_API_KEY = previousOpenAiApiKey; + } + if (previousDatabaseUrl === undefined) { + delete process.env.DATABASE_URL; + } else { + process.env.DATABASE_URL = previousDatabaseUrl; + } + } + + expect(profileIds).toEqual([]); + expect(runtimeProfileIds).toEqual([]); + expect(hostExecApiKey).toBeUndefined(); + expect(hostExecDatabaseUrl).toBeUndefined(); + }); + + it.each([false, true])( + "reads portable shared credentials across temporary state (local override: %s)", + async (localOverride) => { + await withOpenClawTestState( + { scenario: "minimal", env: { OPENAI_API_KEY: undefined } }, + async (state) => { + writeConfigMachineState("auth.sharedStore", { location: "state-db" }); + const sharedStore = { + version: 1, + order: { openai: ["openai:token", "openai:shared", "openai:private"] }, + usageStats: { + "openai:token": { disabledUntil: Date.now() + 3_600_000 }, + "openai:private": { lastUsed: 123 }, + }, + profiles: { + "openai:shared": { + type: "api_key" as const, + provider: "openai", + key: "shared-test-key", + }, + "openai:token": { + type: "token" as const, + provider: "openai", + token: "shared-test-token", + }, + "openai:private": { + type: "api_key" as const, + provider: "openai", + key: "private-test-key", + copyToAgents: false, + }, + "openai:oauth": { + type: "oauth" as const, + provider: "openai", + access: "test-access", + refresh: "test-refresh", + expires: Date.now() + 60_000, + copyToAgents: true, + }, + }, + }; + writePersistedAuthProfileStoreRaw(sharedStore); + if (localOverride) { + writePersistedAuthProfileStoreRaw( + { + version: 1, + profiles: { + "openai:shared": { type: "api_key", provider: "openai", key: "local-test-key" }, + }, + }, + state.agentDir(), + ); + } + setRuntimeAuthProfileStoreSnapshot(sharedStore, state.agentDir()); + const { runtime } = createRuntime(); + let resolvedKey: string | undefined; + const result = await agentExecCommand("inspect", {}, runtime, { + runAgent: async () => { + expect(process.env.OPENCLAW_STATE_DIR).not.toBe(state.stateDir); + const store = ensureAuthProfileStore(undefined, { + externalCli: { mode: "none" }, + syncExternalCli: false, + }); + expect(Object.keys(store.profiles).toSorted()).toEqual([ + "openai:shared", + "openai:token", + ]); + expect( + loadAuthProfileStoreForRuntime(undefined, { + externalCli: { mode: "none" }, + syncExternalCli: false, + }).profiles, + ).toEqual(store.profiles); + expect(findPersistedAuthProfileCredential({ profileId: "openai:token" })).toEqual( + sharedStore.profiles["openai:token"], + ); + expect(store).toMatchObject({ + runtimeLocalProfileIds: localOverride ? ["openai:shared"] : [], + }); + expect(resolveAuthProfileOrder({ store, provider: "openai", cfg: {} })).toEqual([ + "openai:shared", + "openai:token", + ]); + expect(store.usageStats).toEqual({ + "openai:token": sharedStore.usageStats["openai:token"], + }); + const { resolveApiKeyForProfile, saveAuthProfileStore } = + await import("../agents/auth-profiles.js"); + resolvedKey = ( + await resolveApiKeyForProfile({ store, profileId: "openai:shared", cfg: {} }) + )?.apiKey; + expect(inspectPersistedAuthProfileStoreRaw(state.agentDir()).status).toBe( + localOverride ? "readable" : "missing", + ); + saveAuthProfileStore(store); + return successResult(); + }, + }); + expect(result.envelope.error).toBeUndefined(); + expect(resolvedKey).toBe(localOverride ? "local-test-key" : "shared-test-key"); + expect(readPersistedAuthProfileStoreRaw()).toEqual(sharedStore); + expect(readPersistedAuthProfileStoreRaw(state.agentDir())).toEqual({ + version: 1, + profiles: localOverride + ? { + "openai:shared": { type: "api_key", provider: "openai", key: "local-test-key" }, + } + : {}, + }); + }, + ); + }, + ); + + it("rejects an unreadable original shared store before entering temporary exec", async () => { + await withOpenClawTestState({ scenario: "minimal", layout: "split" }, async (state) => { + writeConfigMachineState("auth.sharedStore", { location: "state-db" }); + writePersistedAuthProfileStoreRaw({ version: 1, profiles: "invalid" }); + const { runtime } = createRuntime(); + const runAgent = vi.fn(async () => successResult()); + const result = await agentExecCommand("inspect", {}, runtime, { runAgent }); + expect(result.envelope.error?.message).toContain( + path.join(state.stateDir, "state", "openclaw.sqlite"), + ); + expect(result.envelope.error?.message).toContain("is unreadable; run openclaw doctor --fix"); + expect(runAgent).not.toHaveBeenCalled(); + }); + }); + + it("reads stored credentials from the configured agent directory", async () => { + const stateDir = tempDirs.make("openclaw-agent-exec-cfg-auth-"); + const customAgentDir = path.join(stateDir, "custom-home"); + await fs.mkdir(customAgentDir, { recursive: true }); + const seedPath = path.join(stateDir, "openclaw.json"); + await fs.writeFile( + seedPath, + JSON.stringify({ + agents: { entries: { main: { agentDir: customAgentDir } } }, + }), + "utf8", + ); + const { saveAuthProfileStore } = await import("../agents/auth-profiles.js"); + saveAuthProfileStore( + { + version: 1, + profiles: { "openai:stored": { type: "api_key", provider: "openai", key: "test-key" } }, + }, + customAgentDir, + ); + const { runtime } = createRuntime(); + let scopedProfileIds: string[] = []; + + await agentExecCommand("inspect", { config: seedPath }, runtime, { + runAgent: vi.fn(async () => { + scopedProfileIds = Object.keys(loadAuthProfileStoreForRuntime()?.profiles ?? {}); + return successResult(); + }), + }); + + // The run config strips agentDir to keep run state ephemeral, but credential + // ownership must still follow the operator's configured directory. + expect(scopedProfileIds).toContain("openai:stored"); + }); + + it("blocks direct persisted credential reads under --auth-env-only", async () => { + const normalStateDir = tempDirs.make("openclaw-agent-exec-hidden-auth-"); + const normalAgentDir = path.join(normalStateDir, "agents", "main", "agent"); + const previousStateDir = process.env.OPENCLAW_STATE_DIR; + process.env.OPENCLAW_STATE_DIR = normalStateDir; + const { saveAuthProfileStore } = await import("../agents/auth-profiles.js"); + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai:stored": { type: "api_key", provider: "openai", key: "test-key" }, + }, + }, + normalAgentDir, + ); + const { runtime } = createRuntime(); + let persistedCredential: unknown; + let ownerAgentDir: string | undefined; + try { + await agentExecCommand("inspect", { authEnvOnly: true }, runtime, { + runAgent: vi.fn(async () => { + persistedCredential = findPersistedAuthProfileCredential({ + agentDir: normalAgentDir, + profileId: "openai:stored", + }); + ownerAgentDir = resolvePersistedAuthProfileOwnerAgentDir({ + agentDir: normalAgentDir, + profileId: "openai:stored", + }); + return successResult(); + }), + }); + } finally { + if (previousStateDir === undefined) { + delete process.env.OPENCLAW_STATE_DIR; + } else { + process.env.OPENCLAW_STATE_DIR = previousStateDir; + } + } + + expect(persistedCredential).toBeUndefined(); + expect(ownerAgentDir).toBeUndefined(); + }); + + it("uses the normal stored auth profile when auth-env-only is disabled", async () => { + const normalStateDir = tempDirs.make("openclaw-agent-exec-normal-state-"); + const normalAgentDir = path.join(normalStateDir, "agents", "main", "agent"); + const previousStateDir = process.env.OPENCLAW_STATE_DIR; + process.env.OPENCLAW_STATE_DIR = normalStateDir; + const { saveAuthProfileStore } = await import("../agents/auth-profiles.js"); + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai:stored": { type: "api_key", provider: "openai", key: "test-key" }, + }, + }, + normalAgentDir, + ); + const { runtime } = createRuntime(); + let profileIds: string[] = []; + try { + await agentExecCommand("inspect", { authEnvOnly: false }, runtime, { + runAgent: vi.fn(async () => { + expect(process.env.OPENCLAW_STATE_DIR).not.toBe(normalStateDir); + profileIds = Object.keys( + ensureAuthProfileStore(undefined, { + allowKeychainPrompt: false, + syncExternalCli: false, + }).profiles, + ); + return successResult(); + }), + }); + } finally { + if (previousStateDir === undefined) { + delete process.env.OPENCLAW_STATE_DIR; + } else { + process.env.OPENCLAW_STATE_DIR = previousStateDir; + } + } + + expect(profileIds).toContain("openai:stored"); + }); +}); diff --git a/src/commands/agent-exec.test.ts b/src/commands/agent-exec.test.ts index 012304c8cbfa..b691e77a99d1 100644 --- a/src/commands/agent-exec.test.ts +++ b/src/commands/agent-exec.test.ts @@ -7,12 +7,6 @@ import { promisify } from "node:util"; import { afterEach, describe, expect, it, vi } from "vitest"; import { cleanupTempDirs, useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; import { AgentRunTerminalOutcomeError } from "../agents/agent-run-terminal-error.js"; -import { - ensureAuthProfileStore, - findPersistedAuthProfileCredential, - loadAuthProfileStoreForRuntime, - resolvePersistedAuthProfileOwnerAgentDir, -} from "../agents/auth-profiles.js"; import { enqueueExecutionIdentityContextAtAdmission } from "../audit/execution-identity-admission.js"; import { clearRuntimeConfigSnapshot, @@ -726,196 +720,6 @@ describe("agent exec command composition", () => { // never receive a serialized copy of it. await expect(fs.readdir(stateDir)).resolves.toEqual(["keep.txt"]); }); - - it("skips external Codex CLI credentials under --auth-env-only", async () => { - const codexHome = tempDirs.make("openclaw-agent-exec-codex-home-"); - await fs.writeFile( - path.join(codexHome, "auth.json"), - JSON.stringify({ - auth_mode: "chatgpt", - tokens: { access_token: "test-access", refresh_token: "test-refresh" }, - }), - "utf8", - ); - const previousCodexHome = process.env.CODEX_HOME; - const previousOpenAiApiKey = process.env.OPENAI_API_KEY; - const previousDatabaseUrl = process.env.DATABASE_URL; - process.env.CODEX_HOME = codexHome; - process.env.OPENAI_API_KEY = "test-openai-key"; - process.env.DATABASE_URL = "postgres://test.invalid/database"; - const { runtime } = createRuntime(); - let profileIds: string[] = []; - let runtimeProfileIds: string[] = []; - let hostExecApiKey: string | undefined; - let hostExecDatabaseUrl: string | undefined; - try { - const { withHostExecInheritedEnvOmitted } = await import("../infra/host-env-security.js"); - await withHostExecInheritedEnvOmitted(["DATABASE_URL"], () => - agentExecCommand("inspect", { authEnvOnly: true }, runtime, { - runAgent: vi.fn(async () => { - profileIds = Object.keys( - ensureAuthProfileStore(undefined, { - allowKeychainPrompt: false, - externalCliProviderIds: ["openai"], - }).profiles, - ); - runtimeProfileIds = Object.keys( - loadAuthProfileStoreForRuntime(undefined, { - allowKeychainPrompt: false, - externalCliProviderIds: ["openai"], - }).profiles, - ); - const { sanitizeHostExecEnv } = await import("../infra/host-env-security.js"); - const hostExecEnv = sanitizeHostExecEnv({ baseEnv: process.env }); - hostExecApiKey = hostExecEnv.OPENAI_API_KEY; - hostExecDatabaseUrl = hostExecEnv.DATABASE_URL; - return successResult(); - }), - }), - ); - } finally { - if (previousCodexHome === undefined) { - delete process.env.CODEX_HOME; - } else { - process.env.CODEX_HOME = previousCodexHome; - } - if (previousOpenAiApiKey === undefined) { - delete process.env.OPENAI_API_KEY; - } else { - process.env.OPENAI_API_KEY = previousOpenAiApiKey; - } - if (previousDatabaseUrl === undefined) { - delete process.env.DATABASE_URL; - } else { - process.env.DATABASE_URL = previousDatabaseUrl; - } - } - - expect(profileIds).toEqual([]); - expect(runtimeProfileIds).toEqual([]); - expect(hostExecApiKey).toBeUndefined(); - expect(hostExecDatabaseUrl).toBeUndefined(); - }); - - it("reads stored credentials from the configured agent directory", async () => { - const stateDir = tempDirs.make("openclaw-agent-exec-cfg-auth-"); - const customAgentDir = path.join(stateDir, "custom-home"); - await fs.mkdir(customAgentDir, { recursive: true }); - const seedPath = path.join(stateDir, "openclaw.json"); - await fs.writeFile( - seedPath, - JSON.stringify({ - agents: { entries: { main: { agentDir: customAgentDir } } }, - }), - "utf8", - ); - const { saveAuthProfileStore } = await import("../agents/auth-profiles.js"); - saveAuthProfileStore( - { - version: 1, - profiles: { "openai:stored": { type: "api_key", provider: "openai", key: "test-key" } }, - }, - customAgentDir, - ); - const { runtime } = createRuntime(); - let scopedProfileIds: string[] = []; - - await agentExecCommand("inspect", { config: seedPath }, runtime, { - runAgent: vi.fn(async () => { - scopedProfileIds = Object.keys(loadAuthProfileStoreForRuntime()?.profiles ?? {}); - return successResult(); - }), - }); - - // The run config strips agentDir to keep run state ephemeral, but credential - // ownership must still follow the operator's configured directory. - expect(scopedProfileIds).toContain("openai:stored"); - }); - - it("blocks direct persisted credential reads under --auth-env-only", async () => { - const normalStateDir = tempDirs.make("openclaw-agent-exec-hidden-auth-"); - const normalAgentDir = path.join(normalStateDir, "agents", "main", "agent"); - const previousStateDir = process.env.OPENCLAW_STATE_DIR; - process.env.OPENCLAW_STATE_DIR = normalStateDir; - const { saveAuthProfileStore } = await import("../agents/auth-profiles.js"); - saveAuthProfileStore( - { - version: 1, - profiles: { - "openai:stored": { type: "api_key", provider: "openai", key: "test-key" }, - }, - }, - normalAgentDir, - ); - const { runtime } = createRuntime(); - let persistedCredential: unknown; - let ownerAgentDir: string | undefined; - try { - await agentExecCommand("inspect", { authEnvOnly: true }, runtime, { - runAgent: vi.fn(async () => { - persistedCredential = findPersistedAuthProfileCredential({ - agentDir: normalAgentDir, - profileId: "openai:stored", - }); - ownerAgentDir = resolvePersistedAuthProfileOwnerAgentDir({ - agentDir: normalAgentDir, - profileId: "openai:stored", - }); - return successResult(); - }), - }); - } finally { - if (previousStateDir === undefined) { - delete process.env.OPENCLAW_STATE_DIR; - } else { - process.env.OPENCLAW_STATE_DIR = previousStateDir; - } - } - - expect(persistedCredential).toBeUndefined(); - expect(ownerAgentDir).toBeUndefined(); - }); - - it("uses the normal stored auth profile when auth-env-only is disabled", async () => { - const normalStateDir = tempDirs.make("openclaw-agent-exec-normal-state-"); - const normalAgentDir = path.join(normalStateDir, "agents", "main", "agent"); - const previousStateDir = process.env.OPENCLAW_STATE_DIR; - process.env.OPENCLAW_STATE_DIR = normalStateDir; - const { saveAuthProfileStore } = await import("../agents/auth-profiles.js"); - saveAuthProfileStore( - { - version: 1, - profiles: { - "openai:stored": { type: "api_key", provider: "openai", key: "test-key" }, - }, - }, - normalAgentDir, - ); - const { runtime } = createRuntime(); - let profileIds: string[] = []; - try { - await agentExecCommand("inspect", { authEnvOnly: false }, runtime, { - runAgent: vi.fn(async () => { - expect(process.env.OPENCLAW_STATE_DIR).not.toBe(normalStateDir); - profileIds = Object.keys( - ensureAuthProfileStore(undefined, { - allowKeychainPrompt: false, - syncExternalCli: false, - }).profiles, - ); - return successResult(); - }), - }); - } finally { - if (previousStateDir === undefined) { - delete process.env.OPENCLAW_STATE_DIR; - } else { - process.env.OPENCLAW_STATE_DIR = previousStateDir; - } - } - - expect(profileIds).toContain("openai:stored"); - }); }); describe("agent exec run config layering", () => { diff --git a/src/commands/agent-exec.ts b/src/commands/agent-exec.ts index f3f8c5e163c1..82b5422b14a6 100644 --- a/src/commands/agent-exec.ts +++ b/src/commands/agent-exec.ts @@ -651,8 +651,9 @@ export async function agentExecCommand( // Auth, session keys, and SQLite ownership must share one resolved owner. // Splitting these paths can select an agent's store but emit a `main` key. const storedAuthAgentDir = resolveAgentDir(baseConfig, execAgentId); - restoreEnvironment = setAgentExecEnvironment({ stateDir, cwd }); runtimePaths = await import("../config/paths.js"); + const storedAuthStateDir = runtimePaths.resolveStateDir(); + restoreEnvironment = setAgentExecEnvironment({ stateDir, cwd }); runtimePaths.pinRuntimePaths(); if (opts.stateDir) { const { acquireEmbeddedStateLock, createEmbeddedStateSignalBridge } = @@ -735,7 +736,11 @@ export async function agentExecCommand( const runWithAuthScope = () => opts.authEnvOnly === true ? withEnvOnlyAuthProfileStore(runWithPluginInstallRoots) - : withAuthProfileStoreAgentDir(storedAuthAgentDir, runWithPluginInstallRoots); + : withAuthProfileStoreAgentDir( + storedAuthAgentDir, + storedAuthStateDir, + runWithPluginInstallRoots, + ); const result = await withHostExecInheritedEnvOmitted( listKnownProviderAuthEnvVarNames({ env: process.env }), runWithAuthScope, diff --git a/src/commands/agents.commands.delete.ts b/src/commands/agents.commands.delete.ts index d6b6414153a0..1c500a0176b6 100644 --- a/src/commands/agents.commands.delete.ts +++ b/src/commands/agents.commands.delete.ts @@ -3,6 +3,7 @@ import type { AgentsDeleteResult } from "../../packages/gateway-protocol/src/sch import { findOverlappingWorkspaceAgentIds, formatSharedAuthStoreOwnerDeleteError, + isInheritedAuthStoreOwner, isSharedAuthStoreOwner, } from "../agents/agent-delete-safety.js"; import { @@ -24,7 +25,6 @@ import { resolveSharedAuthStorePath, } from "../agents/auth-profiles/path-resolve.js"; import { resolveAuthProfileDatabasePath } from "../agents/auth-profiles/sqlite.js"; -import { resolveLegacyInheritedAuthAgentId } from "../agents/legacy-inherited-auth-dir.js"; import { prepareLegacyWorkspaceStateReset, removeLegacyWorkspaceStateForReset, @@ -199,11 +199,7 @@ export async function agentsDeleteCommand( ); return; } - const explicitInheritedAuthAgentId = cfg.agents?.defaults?.authInheritance?.agentId?.trim(); - const inheritedAuthAgentId = - explicitInheritedAuthAgentId || - (sharedAuthOwnership.location === "legacy-main" ? resolveLegacyInheritedAuthAgentId(cfg) : ""); - if (inheritedAuthAgentId && agentId === normalizeAgentId(inheritedAuthAgentId)) { + if (isInheritedAuthStoreOwner(cfg, agentId)) { failAgentsDelete( opts, runtime, diff --git a/src/commands/agents.delete.test.ts b/src/commands/agents.delete.test.ts index a46542f3853d..9f93c2bbfac0 100644 --- a/src/commands/agents.delete.test.ts +++ b/src/commands/agents.delete.test.ts @@ -7,6 +7,10 @@ import { toAgentEntriesRecord, tryResolveSoleAgentId, } from "../agents/agent-scope-config.js"; +import { + readPersistedAuthProfileStoreRaw, + writePersistedAuthProfileStoreRaw, +} from "../agents/auth-profiles/sqlite.js"; import { retainLegacyDefaultAgentId, tryGetLegacyDefaultAgentId, @@ -106,6 +110,12 @@ vi.mock("../wizard/clack-prompter.js", () => ({ import { agentsDeleteCommand } from "./agents.commands.delete.js"; const runtime = createTestRuntime(); +const sharedAuthStore = { + version: 1, + profiles: { + "test-provider:shared": { type: "api_key", provider: "test-provider", key: "test-shared-key" }, + }, +}; function gatewayTransportError(kind: "closed" | "timeout", code?: number): GatewayTransportError { return new GatewayTransportError({ @@ -223,7 +233,9 @@ describe("agents delete command", () => { beforeEach(() => { configMocks.readConfigFileSnapshot.mockReset(); configMocks.replaceConfigFile.mockReset(); - fsSafeMocks.movePathToTrash.mockClear(); + fsSafeMocks.movePathToTrash + .mockReset() + .mockImplementation(async (targetPath: string) => `${targetPath}.trashed`); workspaceStateMocks.deleteWorkspaceState.mockClear(); processMocks.runCommandWithTimeout.mockClear(); gatewayMocks.callGateway.mockReset(); @@ -284,6 +296,7 @@ describe("agents delete command", () => { deletedAgentId: "main", sessions, }); + writePersistedAuthProfileStoreRaw(sharedAuthStore, path.join(stateDir, "agents/main/agent")); await agentsDeleteCommand({ id: "main", force: true, json: true }, runtime); expect(gatewayMocks.callGateway).not.toHaveBeenCalled(); @@ -301,6 +314,7 @@ describe("agents delete command", () => { ]); expect(runtime.exit).toHaveBeenCalledWith(1, { resetStream: process.stderr }); expectSessionStore(cfg, sessions, "main"); + expect(readPersistedAuthProfileStoreRaw()).toEqual(sharedAuthStore); }); }); @@ -315,6 +329,7 @@ describe("agents delete command", () => { }, }; writeConfigMachineState("auth.sharedStore", { location: "state-db" }); + writePersistedAuthProfileStoreRaw(sharedAuthStore); await arrangeAgentsDeleteTest({ stateDir, cfg, @@ -331,12 +346,21 @@ describe("agents delete command", () => { ops: { security: "allowlist", allowlist: [{ pattern: "/usr/bin/keep" }] }, }, }); + fsSafeMocks.movePathToTrash.mockImplementation(async (targetPath: string) => { + const trashPath = `${targetPath}.trashed`; + await fs.rename(targetPath, trashPath); + return trashPath; + }); await agentsDeleteCommand({ id: "main", force: true, json: true }, runtime); expect(runtime.error).not.toHaveBeenCalled(); expect(runtime.exit).not.toHaveBeenCalledWith(1); expect(configMocks.replaceConfigFile).toHaveBeenCalledOnce(); + await expect(fs.access(path.join(stateDir, "agents/main/agent"))).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(readPersistedAuthProfileStoreRaw()).toEqual(sharedAuthStore); expectSessionStore(cfg, {}, "main"); expect(readExecApprovalsSnapshot().file.agents).toEqual({ "*": { security: "deny" }, diff --git a/src/gateway/server-methods/agents-mutate.test.ts b/src/gateway/server-methods/agents-mutate.test.ts index f5c1877d62c6..8ea4e4c5d129 100644 --- a/src/gateway/server-methods/agents-mutate.test.ts +++ b/src/gateway/server-methods/agents-mutate.test.ts @@ -3057,21 +3057,43 @@ describe("agents.delete", () => { expect(mocks.movePathToTrash).not.toHaveBeenCalled(); }); - it("deletes main through the normal journal path after shared auth relocation", async () => { + it.each([false, true])( + "deletes main after shared auth relocation (legacy default: %s)", + async (legacyDefault) => { + mocks.sharedAuthStoreOwnership = { location: "state-db" }; + mocks.loadConfigReturn = { + agents: { + list: [{ id: "main" }, { id: "ops", ...(legacyDefault ? { default: true } : {}) }], + }, + }; + + const { respond, promise } = makeCall("agents.delete", { + agentId: "main", + }); + await promise; + + expectRespondOk(respond, { ok: true, agentId: "main" }); + expect(mocks.beginAgentDeletionCommit).toHaveBeenCalledOnce(); + expect(mocks.beginAgentDeletionFinish).toHaveBeenCalledOnce(); + expect(mocks.writeConfigFile).toHaveBeenCalledOnce(); + }, + ); + + it("preserves an explicit inherited auth owner after shared auth relocation", async () => { mocks.sharedAuthStoreOwnership = { location: "state-db" }; mocks.loadConfigReturn = { - agents: { list: [{ id: "main" }, { id: "ops", default: true }] }, + agents: { + defaults: { authInheritance: { agentId: "main" } }, + list: [{ id: "main" }, { id: "ops" }], + }, }; - const { respond, promise } = makeCall("agents.delete", { - agentId: "main", - }); + const { respond, promise } = makeCall("agents.delete", { agentId: "main" }); await promise; - expectRespondOk(respond, { ok: true, agentId: "main" }); - expect(mocks.beginAgentDeletionCommit).toHaveBeenCalledOnce(); - expect(mocks.beginAgentDeletionFinish).toHaveBeenCalledOnce(); - expect(mocks.writeConfigFile).toHaveBeenCalledOnce(); + expectRespondErrorContaining(respond, "owns inherited credentials"); + expect(mocks.beginAgentDeletionCommit).not.toHaveBeenCalled(); + expect(mocks.movePathToTrash).not.toHaveBeenCalled(); }); it("returns not found when a concurrent delete wins the delete race", async () => { diff --git a/src/gateway/server-methods/agents.ts b/src/gateway/server-methods/agents.ts index f4b2b00860a9..c96e9800b2f2 100644 --- a/src/gateway/server-methods/agents.ts +++ b/src/gateway/server-methods/agents.ts @@ -24,6 +24,7 @@ import { createAgent } from "../../agents/agent-create.js"; import { findOverlappingWorkspaceAgentIds, formatSharedAuthStoreOwnerDeleteError, + isInheritedAuthStoreOwner, isSharedAuthStoreOwner, } from "../../agents/agent-delete-safety.js"; import { @@ -57,7 +58,6 @@ import { sanitizeAgentIdentityLine, } from "../../agents/identity-file.js"; import { resolveAgentIdentity } from "../../agents/identity.js"; -import { resolveLegacyInheritedAuthAgentId } from "../../agents/legacy-inherited-auth-dir.js"; import { prepareLegacyWorkspaceStateReset, removeLegacyWorkspaceStateForReset, @@ -1076,8 +1076,7 @@ export const agentsHandlers: GatewayRequestHandlers = { ); return; } - if (agentId === normalizeAgentId(resolveLegacyInheritedAuthAgentId(cfg))) { - // H2-2 owns credential relocation; deleting this directory first destroys the shared store. + if (isInheritedAuthStoreOwner(cfg, agentId)) { respond( false, undefined, @@ -1104,7 +1103,7 @@ export const agentsHandlers: GatewayRequestHandlers = { if (agentId === tryResolveSoleAgentId(lockedConfig)) { throw new AgentConfigPreconditionError(`agent "${agentId}" is the only configured agent`); } - if (agentId === normalizeAgentId(resolveLegacyInheritedAuthAgentId(lockedConfig))) { + if (isInheritedAuthStoreOwner(lockedConfig, agentId)) { throw new AgentConfigPreconditionError( `agent "${agentId}" owns agents.defaults.authInheritance.agentId; relocate credentials and re-point it first`, );