From f46b9c996fb7d84133a2eba736884be3cd76863e Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Wed, 25 Feb 2026 23:25:23 -0600 Subject: [PATCH] feat(secrets): allow opt-in symlink exec command paths --- docs/gateway/configuration-reference.md | 4 +- docs/gateway/secrets.md | 38 +++++++- src/config/config.secrets-schema.test.ts | 1 + src/config/types.secrets.ts | 1 + src/config/zod-schema.core.ts | 1 + src/secrets/configure.ts | 8 ++ src/secrets/plan.ts | 6 ++ src/secrets/resolve.test.ts | 114 +++++++++++++++++++++++ src/secrets/resolve.ts | 77 +++++++++------ 9 files changed, 222 insertions(+), 28 deletions(-) diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index a7f1c378b..27cc236b9 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -2444,7 +2444,9 @@ Validation: Notes: - `file` provider supports `mode: "json"` and `mode: "singleValue"` (`id` must be `"value"` in singleValue mode). -- `exec` provider requires an absolute non-symlink `command` path and uses protocol payloads on stdin/stdout. +- `exec` provider requires an absolute `command` path and uses protocol payloads on stdin/stdout. +- By default, symlink command paths are rejected. Set `allowSymlinkCommand: true` to allow symlink paths while validating the resolved target path. +- If `trustedDirs` is configured, the trusted-dir check applies to the resolved target path. - `exec` child environment is minimal by default; pass required variables explicitly with `passEnv`. - Secret refs are resolved at activation time into an in-memory snapshot, then request paths read the snapshot only. diff --git a/docs/gateway/secrets.md b/docs/gateway/secrets.md index 7269e4d4e..ebeb16cc7 100644 --- a/docs/gateway/secrets.md +++ b/docs/gateway/secrets.md @@ -126,7 +126,9 @@ Define providers under `secrets.providers`: ### Exec provider - Runs configured absolute binary path, no shell. -- `command` must point to a regular file (not a symlink). +- By default, `command` must point to a regular file (not a symlink). +- Set `allowSymlinkCommand: true` to allow symlink command paths (for example Homebrew shims). OpenClaw validates the resolved target path. +- When `trustedDirs` is set, checks apply to the resolved target path. - Supports timeout, no-output timeout, output byte limits, env allowlist, and trusted dirs. - Request payload (stdin): @@ -154,6 +156,37 @@ Optional per-id errors: The patterns below were validated end-to-end with `openclaw secrets audit --json` and `unresolvedRefCount=0`. +### Direct Homebrew command path (no wrapper) + +Use this when your command path is a Homebrew symlink (for example `/opt/homebrew/bin/op`): + +```json5 +{ + secrets: { + providers: { + onepassword_openai: { + source: "exec", + command: "/opt/homebrew/bin/op", + allowSymlinkCommand: true, + trustedDirs: ["/opt/homebrew"], + args: ["read", "op://Personal/OpenClaw QA API Key/password"], + passEnv: ["HOME"], + jsonOnly: false, + }, + }, + }, + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + models: [{ id: "gpt-5", name: "gpt-5" }], + apiKey: { source: "exec", provider: "onepassword_openai", id: "value" }, + }, + }, + }, +} +``` + ### 1Password (`op`) 1. Create a wrapper script (non-symlink command path): @@ -184,6 +217,7 @@ chmod 700 /usr/local/libexec/openclaw/op-openai.sh providers: { openai: { baseUrl: "https://api.openai.com/v1", + models: [{ id: "gpt-5", name: "gpt-5" }], apiKey: { source: "exec", provider: "onepassword_openai", id: "value" }, }, }, @@ -221,6 +255,7 @@ chmod 700 /usr/local/libexec/openclaw/vault-openai.sh providers: { openai: { baseUrl: "https://api.openai.com/v1", + models: [{ id: "gpt-5", name: "gpt-5" }], apiKey: { source: "exec", provider: "vault_openai", id: "value" }, }, }, @@ -258,6 +293,7 @@ chmod 700 /usr/local/libexec/openclaw/sops-openai.sh providers: { openai: { baseUrl: "https://api.openai.com/v1", + models: [{ id: "gpt-5", name: "gpt-5" }], apiKey: { source: "exec", provider: "sops_openai", id: "value" }, }, }, diff --git a/src/config/config.secrets-schema.test.ts b/src/config/config.secrets-schema.test.ts index 43379464d..aa1b9ab8a 100644 --- a/src/config/config.secrets-schema.test.ts +++ b/src/config/config.secrets-schema.test.ts @@ -17,6 +17,7 @@ describe("config secret refs schema", () => { source: "exec", command: "/usr/local/bin/openclaw-secret-resolver", args: ["resolve"], + allowSymlinkCommand: true, }, }, }, diff --git a/src/config/types.secrets.ts b/src/config/types.secrets.ts index 547c04987..5f009f79e 100644 --- a/src/config/types.secrets.ts +++ b/src/config/types.secrets.ts @@ -128,6 +128,7 @@ export type ExecSecretProviderConfig = { passEnv?: string[]; trustedDirs?: string[]; allowInsecurePath?: boolean; + allowSymlinkCommand?: boolean; }; export type SecretProviderConfig = diff --git a/src/config/zod-schema.core.ts b/src/config/zod-schema.core.ts index e2a9f45cf..b30df9d00 100644 --- a/src/config/zod-schema.core.ts +++ b/src/config/zod-schema.core.ts @@ -136,6 +136,7 @@ const SecretsExecProviderSchema = z .max(64) .optional(), allowInsecurePath: z.boolean().optional(), + allowSymlinkCommand: z.boolean().optional(), }) .strict(); diff --git a/src/secrets/configure.ts b/src/secrets/configure.ts index df6d48e48..334d4c1d0 100644 --- a/src/secrets/configure.ts +++ b/src/secrets/configure.ts @@ -513,6 +513,13 @@ async function promptExecProvider( }), "Secrets configure cancelled.", ); + const allowSymlinkCommand = assertNoCancel( + await confirm({ + message: "Allow symlink command path?", + initialValue: base?.allowSymlinkCommand ?? false, + }), + "Secrets configure cancelled.", + ); const args = await parseArgsInput(String(argsRaw ?? "")); const timeoutMs = parseOptionalPositiveInt(String(timeoutMsRaw ?? ""), 120000); @@ -535,6 +542,7 @@ async function promptExecProvider( ...(passEnv.length > 0 ? { passEnv } : {}), ...(trustedDirs.length > 0 ? { trustedDirs } : {}), ...(allowInsecurePath ? { allowInsecurePath: true } : {}), + ...(allowSymlinkCommand ? { allowSymlinkCommand: true } : {}), ...(isRecord(base?.env) ? { env: base.env } : {}), }; } diff --git a/src/secrets/plan.ts b/src/secrets/plan.ts index df25a6901..088c27fc4 100644 --- a/src/secrets/plan.ts +++ b/src/secrets/plan.ts @@ -90,6 +90,12 @@ function isSecretProviderConfigShape(value: unknown): value is SecretProviderCon ) { return false; } + if (value.allowInsecurePath !== undefined && typeof value.allowInsecurePath !== "boolean") { + return false; + } + if (value.allowSymlinkCommand !== undefined && typeof value.allowSymlinkCommand !== "boolean") { + return false; + } if (value.env !== undefined) { if (!isObjectRecord(value.env)) { return false; diff --git a/src/secrets/resolve.test.ts b/src/secrets/resolve.test.ts index b47a788dc..4103e8c34 100644 --- a/src/secrets/resolve.test.ts +++ b/src/secrets/resolve.test.ts @@ -145,6 +145,120 @@ describe("secret ref resolver", () => { expect(value).toBe("plain-secret"); }); + it("rejects symlink command paths unless allowSymlinkCommand is enabled", async () => { + if (process.platform === "win32") { + return; + } + const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-secrets-resolve-exec-link-")); + cleanupRoots.push(root); + const scriptPath = path.join(root, "resolver-target.mjs"); + const symlinkPath = path.join(root, "resolver-link.mjs"); + await writeSecureFile( + scriptPath, + ["#!/usr/bin/env node", "process.stdout.write('plain-secret');"].join("\n"), + 0o700, + ); + await fs.symlink(scriptPath, symlinkPath); + + await expect( + resolveSecretRefString( + { source: "exec", provider: "execmain", id: "openai/api-key" }, + { + config: { + secrets: { + providers: { + execmain: { + source: "exec", + command: symlinkPath, + passEnv: ["PATH"], + jsonOnly: false, + }, + }, + }, + }, + }, + ), + ).rejects.toThrow("must not be a symlink"); + }); + + it("allows symlink command paths when allowSymlinkCommand is enabled", async () => { + if (process.platform === "win32") { + return; + } + const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-secrets-resolve-exec-link-")); + cleanupRoots.push(root); + const scriptPath = path.join(root, "resolver-target.mjs"); + const symlinkPath = path.join(root, "resolver-link.mjs"); + await writeSecureFile( + scriptPath, + ["#!/usr/bin/env node", "process.stdout.write('plain-secret');"].join("\n"), + 0o700, + ); + await fs.symlink(scriptPath, symlinkPath); + const trustedRoot = await fs.realpath(root); + + const value = await resolveSecretRefString( + { source: "exec", provider: "execmain", id: "openai/api-key" }, + { + config: { + secrets: { + providers: { + execmain: { + source: "exec", + command: symlinkPath, + passEnv: ["PATH"], + jsonOnly: false, + allowSymlinkCommand: true, + trustedDirs: [trustedRoot], + }, + }, + }, + }, + }, + ); + expect(value).toBe("plain-secret"); + }); + + it("checks trustedDirs against resolved symlink target", async () => { + if (process.platform === "win32") { + return; + } + const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-secrets-resolve-exec-link-")); + const outside = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-secrets-resolve-exec-out-")); + cleanupRoots.push(root); + cleanupRoots.push(outside); + const scriptPath = path.join(outside, "resolver-target.mjs"); + const symlinkPath = path.join(root, "resolver-link.mjs"); + await writeSecureFile( + scriptPath, + ["#!/usr/bin/env node", "process.stdout.write('plain-secret');"].join("\n"), + 0o700, + ); + await fs.symlink(scriptPath, symlinkPath); + + await expect( + resolveSecretRefString( + { source: "exec", provider: "execmain", id: "openai/api-key" }, + { + config: { + secrets: { + providers: { + execmain: { + source: "exec", + command: symlinkPath, + passEnv: ["PATH"], + jsonOnly: false, + allowSymlinkCommand: true, + trustedDirs: [root], + }, + }, + }, + }, + }, + ), + ).rejects.toThrow("outside trustedDirs"); + }); + it("rejects exec refs when protocolVersion is not 1", async () => { if (process.platform === "win32") { return; diff --git a/src/secrets/resolve.ts b/src/secrets/resolve.ts index 50118bffa..9d81486ac 100644 --- a/src/secrets/resolve.ts +++ b/src/secrets/resolve.ts @@ -102,45 +102,68 @@ async function assertSecurePath(params: { trustedDirs?: string[]; allowInsecurePath?: boolean; allowReadableByOthers?: boolean; -}): Promise { + allowSymlinkPath?: boolean; +}): Promise { if (!isAbsolutePathname(params.targetPath)) { throw new Error(`${params.label} must be an absolute path.`); } - if (params.trustedDirs && params.trustedDirs.length > 0) { - const trusted = params.trustedDirs.map((entry) => resolveUserPath(entry)); - const inTrustedDir = trusted.some((dir) => isPathInside(dir, params.targetPath)); - if (!inTrustedDir) { - throw new Error(`${params.label} is outside trustedDirs: ${params.targetPath}`); + + let effectivePath = params.targetPath; + let stat = await safeStat(effectivePath); + if (!stat.ok) { + throw new Error(`${params.label} is not readable: ${effectivePath}`); + } + if (stat.isDir) { + throw new Error(`${params.label} must be a file: ${effectivePath}`); + } + if (stat.isSymlink) { + if (!params.allowSymlinkPath) { + throw new Error(`${params.label} must not be a symlink: ${effectivePath}`); + } + try { + effectivePath = await fs.realpath(effectivePath); + } catch { + throw new Error(`${params.label} symlink target is not readable: ${params.targetPath}`); + } + if (!isAbsolutePathname(effectivePath)) { + throw new Error(`${params.label} resolved symlink target must be an absolute path.`); + } + stat = await safeStat(effectivePath); + if (!stat.ok) { + throw new Error(`${params.label} is not readable: ${effectivePath}`); + } + if (stat.isDir) { + throw new Error(`${params.label} must be a file: ${effectivePath}`); + } + if (stat.isSymlink) { + throw new Error(`${params.label} symlink target must not be a symlink: ${effectivePath}`); } } - const stat = await safeStat(params.targetPath); - if (!stat.ok) { - throw new Error(`${params.label} is not readable: ${params.targetPath}`); - } - if (stat.isDir) { - throw new Error(`${params.label} must be a file: ${params.targetPath}`); - } - if (stat.isSymlink) { - throw new Error(`${params.label} must not be a symlink: ${params.targetPath}`); + if (params.trustedDirs && params.trustedDirs.length > 0) { + const trusted = params.trustedDirs.map((entry) => resolveUserPath(entry)); + const inTrustedDir = trusted.some((dir) => isPathInside(dir, effectivePath)); + if (!inTrustedDir) { + throw new Error(`${params.label} is outside trustedDirs: ${effectivePath}`); + } } if (params.allowInsecurePath) { - return; + return effectivePath; } - const perms = await inspectPathPermissions(params.targetPath); + const perms = await inspectPathPermissions(effectivePath); if (!perms.ok) { - throw new Error(`${params.label} permissions could not be verified: ${params.targetPath}`); + throw new Error(`${params.label} permissions could not be verified: ${effectivePath}`); } const writableByOthers = perms.worldWritable || perms.groupWritable; const readableByOthers = perms.worldReadable || perms.groupReadable; if (writableByOthers || (!params.allowReadableByOthers && readableByOthers)) { - throw new Error(`${params.label} permissions are too open: ${params.targetPath}`); + throw new Error(`${params.label} permissions are too open: ${effectivePath}`); } if (process.platform === "win32" && perms.source === "unknown") { throw new Error( - `${params.label} ACL verification unavailable on Windows for ${params.targetPath}.`, + `${params.label} ACL verification unavailable on Windows for ${effectivePath}.`, ); } @@ -148,10 +171,11 @@ async function assertSecurePath(params: { const uid = process.getuid(); if (stat.uid !== uid) { throw new Error( - `${params.label} must be owned by the current user (uid=${uid}): ${params.targetPath}`, + `${params.label} must be owned by the current user (uid=${uid}): ${effectivePath}`, ); } } + return effectivePath; } async function readFileProviderPayload(params: { @@ -167,7 +191,7 @@ async function readFileProviderPayload(params: { const filePath = resolveUserPath(params.providerConfig.path); const readPromise = (async () => { - await assertSecurePath({ + const secureFilePath = await assertSecurePath({ targetPath: filePath, label: `secrets.providers.${params.providerName}.path`, }); @@ -187,7 +211,7 @@ async function readFileProviderPayload(params: { }); try { const payload = await Promise.race([ - fs.readFile(filePath, { signal: abortController.signal }), + fs.readFile(secureFilePath, { signal: abortController.signal }), timeoutPromise, ]); if (payload.byteLength > maxBytes) { @@ -459,12 +483,13 @@ async function resolveExecRefs(params: { } const commandPath = resolveUserPath(params.providerConfig.command); - await assertSecurePath({ + const secureCommandPath = await assertSecurePath({ targetPath: commandPath, label: `secrets.providers.${params.providerName}.command`, trustedDirs: params.providerConfig.trustedDirs, allowInsecurePath: params.providerConfig.allowInsecurePath, allowReadableByOthers: true, + allowSymlinkPath: params.providerConfig.allowSymlinkCommand, }); const requestPayload = { @@ -502,9 +527,9 @@ async function resolveExecRefs(params: { const jsonOnly = params.providerConfig.jsonOnly ?? true; const result = await runExecResolver({ - command: commandPath, + command: secureCommandPath, args: params.providerConfig.args ?? [], - cwd: path.dirname(commandPath), + cwd: path.dirname(secureCommandPath), env: childEnv, input, timeoutMs,