diff --git a/docs/cli/secrets.md b/docs/cli/secrets.md index 2bb971d0b993..967e04c32338 100644 --- a/docs/cli/secrets.md +++ b/docs/cli/secrets.md @@ -92,6 +92,8 @@ openclaw secrets store get LOG_LEVEL Secret values never appear in human, `--json`, or `--plain` output. `store get` refuses a `secret` entry as write-only by design and exits `2`; it exits `3` when the name does not exist. Environment-kind values are readable. +Team-scoped `env` entries also reach agent exec environments. Explicit per-call env wins over store values, and host/sandbox security filters can reject protected or credential-shaped names with a warning. `secret` entries are never exposed as subprocess env; use them through `store` SecretRefs instead. + ### Remove values ```bash diff --git a/docs/gateway/secrets.md b/docs/gateway/secrets.md index 788ff8691915..612c0222f75a 100644 --- a/docs/gateway/secrets.md +++ b/docs/gateway/secrets.md @@ -276,7 +276,9 @@ The shared secret store is a Gateway-wide, team-scoped place for secrets and env Entries have a `secret` or `env` kind. The kind controls CLI disclosure, not SecretRef resolution: - `secret` values are write-only through the CLI. List and get output never reveal them. -- `env` values can be returned by `store list` and `store get`. +- `env` values can be returned by `store list` and `store get`. Team-scoped `env` entries are also added to agent exec environments, after inherited process values and before explicit per-call env. Protected host keys and sandbox-blocked credential names are ignored with a visible warning. + +`secret` entries are never injected into subprocess environments. They remain available only through `store` SecretRefs because plaintext env injection would bypass the store disclosure boundary; safe secret injection requires a future egress-substitution mechanism. Names use the same uppercase grammar as env SecretRefs, and each UTF-8 value is limited to 64 KiB (65,536 bytes). This supports PEM keys and service-account JSON without inheriting the smaller limits of ordinary environment variables. diff --git a/src/agents/bash-tools.exec-request-preparation.ts b/src/agents/bash-tools.exec-request-preparation.ts index b573887cb1ee..329a7afe57e6 100644 --- a/src/agents/bash-tools.exec-request-preparation.ts +++ b/src/agents/bash-tools.exec-request-preparation.ts @@ -23,6 +23,7 @@ import type { ExecToolDefaults } from "./bash-tools.exec-types.js"; import { type ExecWorkdirResolution, resolveExecWorkdir } from "./bash-tools.exec-workdir.js"; import { buildSandboxEnv, coerceEnv } from "./bash-tools.shared.js"; import type { BashSandboxConfig } from "./bash-tools.shared.js"; +import { sanitizeEnvVars } from "./sandbox/sanitize-env-vars.js"; export type ExecToolArgs = Record & { command: string; @@ -353,16 +354,54 @@ export function resolvePreparedExecEnvironment(params: { channelContext?: PluginHookChannelContext; defaultPathPrepend: string[]; pluginEnv?: Record; + storeEnv?: Record; warnings: string[]; }): { env: Record; requestedEnv?: Record } { const inheritedBaseEnv = coerceEnv(process.env); const channelContextEnv = buildChannelContextEnv(params.channelContext); - const requestedEnv: Record | undefined = + const explicitEnv: Record | undefined = params.execParams.env !== undefined || params.pluginEnv !== undefined || channelContextEnv !== undefined ? { ...params.execParams.env, ...params.pluginEnv, ...channelContextEnv } : undefined; + const storeEnvResult = params.storeEnv + ? sanitizeHostExecEnvWithDiagnostics({ + baseEnv: {}, + overrides: params.storeEnv, + blockPathOverrides: true, + }) + : undefined; + const { [OPENCLAW_CLI_ENV_VAR]: _storeMarker, ...acceptedStoreEnv } = storeEnvResult?.env ?? {}; + let storeEnv = Object.keys(acceptedStoreEnv).length > 0 ? acceptedStoreEnv : undefined; + const rejectedStoreKeys = new Set([ + ...(storeEnvResult?.rejectedOverrideBlockedKeys ?? []), + ...(storeEnvResult?.rejectedOverrideInvalidKeys ?? []), + ]); + if (params.storeEnv && Object.hasOwn(params.storeEnv, OPENCLAW_CLI_ENV_VAR)) { + rejectedStoreKeys.add(OPENCLAW_CLI_ENV_VAR); + } + if (params.host === "sandbox" && storeEnv) { + const sandboxStoreEnvResult = sanitizeEnvVars(storeEnv); + storeEnv = sandboxStoreEnvResult.allowed; + for (const key of sandboxStoreEnvResult.blocked) { + rejectedStoreKeys.add(key); + } + if (sandboxStoreEnvResult.warnings.length > 0) { + params.warnings.push( + `Warning: secret store environment entries need attention: ${sandboxStoreEnvResult.warnings.join("; ")}.`, + ); + } + } + if (rejectedStoreKeys.size > 0) { + params.warnings.push( + `Warning: secret store environment entries were not applied for host=${params.host}: ${Array.from(rejectedStoreKeys).toSorted().join(", ")}.`, + ); + } + const hasStoreEnv = storeEnv && Object.keys(storeEnv).length > 0; + const requestedEnv: Record | undefined = hasStoreEnv + ? { ...storeEnv, ...explicitEnv } + : explicitEnv; const hostEnvResult = params.host === "sandbox" ? null diff --git a/src/agents/bash-tools.exec-run.ts b/src/agents/bash-tools.exec-run.ts index 2a80cd4a2969..12ab94bb3a4d 100644 --- a/src/agents/bash-tools.exec-run.ts +++ b/src/agents/bash-tools.exec-run.ts @@ -66,6 +66,21 @@ import type { AgentToolWithMeta } from "./tools/common.js"; export function createExecTool( defaults?: ExecToolDefaults, ): AgentToolWithMeta { + // Agent runs own one tool instance, so the store is read on first exec and reused for that run. + // A new run constructs a new instance and observes later store mutations. + let storeEnvPromise: Promise | undefined> | undefined; + const resolveStoreEnv = () => { + storeEnvPromise ??= import("../secrets/store/secret-store.js").then((store) => { + const env: Record = {}; + for (const entry of store.listSecretStoreEntries({ scope: { kind: "team" } })) { + if (entry.kind === "env" && entry.valuePreview !== undefined) { + env[entry.name] = entry.valuePreview; + } + } + return Object.keys(env).length > 0 ? env : undefined; + }); + return storeEnvPromise; + }; const defaultBackgroundMs = clampWithDefault( defaults?.backgroundMs ?? readEnvInt("OPENCLAW_BASH_YIELD_MS", "PI_BASH_YIELD_MS"), 10_000, @@ -382,6 +397,7 @@ export function createExecTool( } const resolvedExecEnvState = requestPreparation.getResolvedExecEnvPreparedState(params); + const storeEnv = await resolveStoreEnv(); const { env, requestedEnv } = resolvePreparedExecEnvironment({ execParams: params, host, @@ -390,6 +406,7 @@ export function createExecTool( channelContext: defaults?.channelContext, defaultPathPrepend, pluginEnv: resolvedExecEnvState?.pluginEnv, + storeEnv, warnings, }); diff --git a/src/agents/bash-tools.exec.store-env.test.ts b/src/agents/bash-tools.exec.store-env.test.ts new file mode 100644 index 000000000000..14a835f29101 --- /dev/null +++ b/src/agents/bash-tools.exec.store-env.test.ts @@ -0,0 +1,247 @@ +/** Store-backed exec environment tests cover run snapshots, precedence, and security filtering. */ +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { createTempDirTracker } from "../../test/helpers/temp-dir.js"; +import { writeSecretStoreEntry } from "../secrets/store/secret-store.js"; +import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; +import { captureEnv } from "../test-utils/env.js"; +import type { BashSandboxConfig } from "./bash-tools.shared.js"; + +const mocks = vi.hoisted(() => ({ + gatewayParams: [] as Array<{ + env: Record; + requestedEnv?: Record; + }>, + spawnInputs: [] as Array<{ env?: Record }>, +})); + +vi.mock("../plugins/hook-runner-global.js", () => ({ + getGlobalHookRunner: () => null, + getGlobalHookRunnerRegistry: () => null, +})); + +vi.mock("../infra/shell-env.js", () => ({ + getShellEnvAppliedKeys: vi.fn(() => []), + getShellPathFromLoginShell: vi.fn(() => null), + resolveShellEnvFallbackTimeoutMs: vi.fn(() => 0), + shouldDeferShellEnvFallback: vi.fn(() => false), + shouldEnableShellEnvFallback: vi.fn(() => false), +})); + +vi.mock("./bash-tools.exec-host-gateway.js", () => ({ + processGatewayAllowlist: vi.fn( + async (params: { env: Record; requestedEnv?: Record }) => { + mocks.gatewayParams.push({ + env: { ...params.env }, + requestedEnv: params.requestedEnv ? { ...params.requestedEnv } : undefined, + }); + return {}; + }, + ), +})); + +vi.mock("../process/supervisor/index.js", () => ({ + getProcessSupervisor: () => ({ + spawn: async (input: { env?: Record; onStdout?: (chunk: string) => void }) => { + mocks.spawnInputs.push({ env: input.env ? { ...input.env } : undefined }); + input.onStdout?.("ok\n"); + return { + runId: "mock-run", + startedAtMs: Date.now(), + stdin: undefined, + wait: async () => ({ + reason: "exit" as const, + exitCode: 0, + exitSignal: null, + durationMs: 0, + stdout: "", + stderr: "", + timedOut: false, + noOutputTimedOut: false, + }), + cancel: vi.fn(), + }; + }, + cancel: vi.fn(), + cancelScope: vi.fn(), + getRecord: vi.fn(), + }), +})); + +let createExecTool: typeof import("./bash-tools.exec-run.js").createExecTool; +let createLazyExecTool: typeof import("./lazy-exec-tool.js").createLazyExecTool; + +type StoreEntry = { name: string; value: string; kind: "env" | "secret" }; + +async function withTeamStoreEntries( + entries: StoreEntry[], + run: () => Promise, +): Promise { + const tempDirs = createTempDirTracker(); + const stateDir = tempDirs.make("openclaw-exec-store-env-"); + const envSnapshot = captureEnv(["OPENCLAW_STATE_DIR"]); + process.env.OPENCLAW_STATE_DIR = stateDir; + try { + for (const entry of entries) { + writeSecretStoreEntry({ scope: { kind: "team" }, ...entry, updatedBy: "test" }); + } + await run(); + } finally { + closeOpenClawStateDatabaseForTest(); + envSnapshot.restore(); + tempDirs.cleanup(); + } +} + +describe("exec store environment", () => { + beforeAll(async () => { + ({ createExecTool } = await import("./bash-tools.exec-run.js")); + ({ createLazyExecTool } = await import("./lazy-exec-tool.js")); + }); + + beforeEach(() => { + mocks.gatewayParams.length = 0; + mocks.spawnInputs.length = 0; + }); + + it("adds only team env-kind entries to gateway exec subprocesses", async () => { + await withTeamStoreEntries( + [ + { name: "AWS_REGION", value: "us-west-2", kind: "env" }, + { name: "INTERNAL_VALUE", value: "not-for-subprocesses", kind: "secret" }, + ], + async () => { + const tool = createLazyExecTool({ host: "gateway", security: "full", ask: "off" }); + + await tool.execute("call-store-env", { command: "echo ok", yieldMs: 120_000 }); + + expect(mocks.gatewayParams[0]?.env.AWS_REGION).toBe("us-west-2"); + expect(mocks.gatewayParams[0]?.env).not.toHaveProperty("INTERNAL_VALUE"); + }, + ); + }); + + it("lets explicitly requested env override a store entry", async () => { + await withTeamStoreEntries( + [{ name: "AWS_REGION", value: "us-west-2", kind: "env" }], + async () => { + const tool = createLazyExecTool({ host: "gateway", security: "full", ask: "off" }); + + await tool.execute("call-store-env-override", { + command: "echo ok", + env: { AWS_REGION: "eu-central-1" }, + yieldMs: 120_000, + }); + + expect(mocks.gatewayParams[0]?.env.AWS_REGION).toBe("eu-central-1"); + expect(mocks.gatewayParams[0]?.requestedEnv?.AWS_REGION).toBe("eu-central-1"); + }, + ); + }); + + it("ignores protected store entries without replacing inherited network settings", async () => { + const envSnapshot = captureEnv(["PATH", "HTTPS_PROXY", "NODE_EXTRA_CA_CERTS"]); + process.env.PATH = "/inherited/bin"; + process.env.HTTPS_PROXY = "http://inherited-proxy.test:8080"; + process.env.NODE_EXTRA_CA_CERTS = "/inherited/ca.pem"; + try { + await withTeamStoreEntries( + [ + { name: "PATH", value: "/store/bin", kind: "env" }, + { name: "HTTPS_PROXY", value: "http://store-proxy.test:8080", kind: "env" }, + { name: "NODE_EXTRA_CA_CERTS", value: "/store/ca.pem", kind: "env" }, + ], + async () => { + const tool = createLazyExecTool({ host: "gateway", security: "full", ask: "off" }); + + const result = await tool.execute("call-protected-store-env", { + command: "echo ok", + yieldMs: 120_000, + }); + + expect(mocks.gatewayParams[0]?.env).toMatchObject({ + PATH: "/inherited/bin", + HTTPS_PROXY: "http://inherited-proxy.test:8080", + NODE_EXTRA_CA_CERTS: "/inherited/ca.pem", + }); + expect(mocks.gatewayParams[0]?.requestedEnv).toBeUndefined(); + expect(result.content[0]).toMatchObject({ + type: "text", + text: expect.stringMatching(/HTTPS_PROXY, NODE_EXTRA_CA_CERTS, PATH/u), + }); + }, + ); + } finally { + envSnapshot.restore(); + } + }); + + it("filters sandbox store env and surfaces credential-shaped drops", async () => { + await withTeamStoreEntries( + [ + { name: "AWS_REGION", value: "us-west-2", kind: "env" }, + { name: "FOO_TOKEN", value: "operator-forced-env", kind: "env" }, + ], + async () => { + const buildExecSpec = vi.fn>( + async (params) => ({ + argv: ["remote-shell", params.command], + env: {}, + stdinMode: "pipe-open" as const, + }), + ); + const tool = createLazyExecTool({ + host: "sandbox", + security: "full", + ask: "off", + cwd: process.cwd(), + sandbox: { + containerName: "store-env-sandbox", + workspaceDir: process.cwd(), + containerWorkdir: "/workspace", + buildExecSpec, + }, + }); + + const result = await tool.execute("call-sandbox-store-env", { + command: "echo ok", + yieldMs: 120_000, + }); + + expect(buildExecSpec.mock.calls[0]?.[0]?.env).toMatchObject({ AWS_REGION: "us-west-2" }); + expect(buildExecSpec.mock.calls[0]?.[0]?.env).not.toHaveProperty("FOO_TOKEN"); + expect(result.content[0]).toMatchObject({ + type: "text", + text: expect.stringContaining("FOO_TOKEN"), + }); + }, + ); + }); + + it("keeps an empty store snapshot byte-identical to direct exec env assembly", async () => { + await withTeamStoreEntries([], async () => { + const directTool = createExecTool({ host: "gateway", security: "full", ask: "off" }); + await directTool.execute("call-direct-empty-store-baseline", { + command: "echo ok", + env: { REQUEST_SAFE: "request" }, + yieldMs: 120_000, + }); + const baseline = JSON.stringify({ + gateway: mocks.gatewayParams[0], + spawn: mocks.spawnInputs[0], + }); + mocks.gatewayParams.length = 0; + mocks.spawnInputs.length = 0; + + const lazyTool = createLazyExecTool({ host: "gateway", security: "full", ask: "off" }); + await lazyTool.execute("call-lazy-empty-store", { + command: "echo ok", + env: { REQUEST_SAFE: "request" }, + yieldMs: 120_000, + }); + + expect(JSON.stringify({ gateway: mocks.gatewayParams[0], spawn: mocks.spawnInputs[0] })).toBe( + baseline, + ); + }); + }); +}); diff --git a/src/agents/lazy-exec-tool.ts b/src/agents/lazy-exec-tool.ts index 5511d2387b25..9752b0eaaf31 100644 --- a/src/agents/lazy-exec-tool.ts +++ b/src/agents/lazy-exec-tool.ts @@ -26,12 +26,16 @@ export function createLazyExecTool( presentation?: LazyExecToolPresentation, ): AnyAgentTool { let loadedTool: AnyAgentTool | undefined; - const loadTool = async () => { - if (!loadedTool) { - const { createExecTool } = await bashToolsModuleLoader.load(); - loadedTool = createExecTool(defaults) as unknown as AnyAgentTool; + let loadingTool: Promise | undefined; + const loadTool = () => { + if (loadedTool) { + return Promise.resolve(loadedTool); } - return loadedTool; + loadingTool ??= bashToolsModuleLoader.load().then(({ createExecTool }) => { + loadedTool = createExecTool(defaults) as unknown as AnyAgentTool; + return loadedTool; + }); + return loadingTool; }; return {