feat(secrets): allow opt-in symlink exec command paths
This commit is contained in:
committed by
Peter Steinberger
parent
06290b49b2
commit
f46b9c996f
@@ -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.
|
||||
|
||||
|
||||
@@ -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" },
|
||||
},
|
||||
},
|
||||
|
||||
@@ -17,6 +17,7 @@ describe("config secret refs schema", () => {
|
||||
source: "exec",
|
||||
command: "/usr/local/bin/openclaw-secret-resolver",
|
||||
args: ["resolve"],
|
||||
allowSymlinkCommand: true,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -128,6 +128,7 @@ export type ExecSecretProviderConfig = {
|
||||
passEnv?: string[];
|
||||
trustedDirs?: string[];
|
||||
allowInsecurePath?: boolean;
|
||||
allowSymlinkCommand?: boolean;
|
||||
};
|
||||
|
||||
export type SecretProviderConfig =
|
||||
|
||||
@@ -136,6 +136,7 @@ const SecretsExecProviderSchema = z
|
||||
.max(64)
|
||||
.optional(),
|
||||
allowInsecurePath: z.boolean().optional(),
|
||||
allowSymlinkCommand: z.boolean().optional(),
|
||||
})
|
||||
.strict();
|
||||
|
||||
|
||||
@@ -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 } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -102,45 +102,68 @@ async function assertSecurePath(params: {
|
||||
trustedDirs?: string[];
|
||||
allowInsecurePath?: boolean;
|
||||
allowReadableByOthers?: boolean;
|
||||
}): Promise<void> {
|
||||
allowSymlinkPath?: boolean;
|
||||
}): Promise<string> {
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user