mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
feat(secrets): inject store env into agent exec (#121773)
This commit is contained in:
committed by
GitHub
parent
8aa0376f27
commit
52e3149ed7
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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<string, unknown> & {
|
||||
command: string;
|
||||
@@ -353,16 +354,54 @@ export function resolvePreparedExecEnvironment(params: {
|
||||
channelContext?: PluginHookChannelContext;
|
||||
defaultPathPrepend: string[];
|
||||
pluginEnv?: Record<string, string>;
|
||||
storeEnv?: Record<string, string>;
|
||||
warnings: string[];
|
||||
}): { env: Record<string, string>; requestedEnv?: Record<string, string> } {
|
||||
const inheritedBaseEnv = coerceEnv(process.env);
|
||||
const channelContextEnv = buildChannelContextEnv(params.channelContext);
|
||||
const requestedEnv: Record<string, string> | undefined =
|
||||
const explicitEnv: Record<string, string> | 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<string, string> | undefined = hasStoreEnv
|
||||
? { ...storeEnv, ...explicitEnv }
|
||||
: explicitEnv;
|
||||
const hostEnvResult =
|
||||
params.host === "sandbox"
|
||||
? null
|
||||
|
||||
@@ -66,6 +66,21 @@ import type { AgentToolWithMeta } from "./tools/common.js";
|
||||
export function createExecTool(
|
||||
defaults?: ExecToolDefaults,
|
||||
): AgentToolWithMeta<typeof execSchema, ExecToolDetails> {
|
||||
// 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<Record<string, string> | undefined> | undefined;
|
||||
const resolveStoreEnv = () => {
|
||||
storeEnvPromise ??= import("../secrets/store/secret-store.js").then((store) => {
|
||||
const env: Record<string, string> = {};
|
||||
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,
|
||||
});
|
||||
|
||||
|
||||
@@ -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<string, string>;
|
||||
requestedEnv?: Record<string, string>;
|
||||
}>,
|
||||
spawnInputs: [] as Array<{ env?: Record<string, string> }>,
|
||||
}));
|
||||
|
||||
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<string, string>; requestedEnv?: Record<string, string> }) => {
|
||||
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<string, string>; 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<void>,
|
||||
): Promise<void> {
|
||||
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<NonNullable<BashSandboxConfig["buildExecSpec"]>>(
|
||||
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,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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<AnyAgentTool> | 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 {
|
||||
|
||||
Reference in New Issue
Block a user