mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(secrets): harden exec provider diagnostics (#105082)
This commit is contained in:
committed by
GitHub
parent
be74297a14
commit
b6c171901e
@@ -238,10 +238,15 @@ Optional per-id errors:
|
||||
{
|
||||
"protocolVersion": 1,
|
||||
"values": {},
|
||||
"errors": { "providers/openai/apiKey": { "message": "not found" } }
|
||||
"errors": { "providers/openai/apiKey": { "code": "NOT_FOUND" } }
|
||||
}
|
||||
```
|
||||
|
||||
`code` is an optional machine-readable diagnostic. OpenClaw displays the recognized
|
||||
codes `NOT_FOUND` and `AMBIGUOUS_DUPLICATE_KEY` with the provider and ref id. Other
|
||||
codes and free-form fields such as `message` are accepted for protocol-v1 compatibility
|
||||
but are not displayed because resolver output can contain credential material.
|
||||
|
||||
</Accordion>
|
||||
|
||||
## File-backed API keys
|
||||
|
||||
@@ -81,9 +81,9 @@ const main = async () => {
|
||||
if (matches.length === 1) {
|
||||
values[id] = matches[0];
|
||||
} else if (matches.length > 1) {
|
||||
errors[id] = { message: "ambiguous duplicate key" };
|
||||
errors[id] = { code: "AMBIGUOUS_DUPLICATE_KEY" };
|
||||
} else {
|
||||
errors[id] = { message: "not found" };
|
||||
errors[id] = { code: "NOT_FOUND" };
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -58,6 +58,8 @@ describe("secret ref resolver", () => {
|
||||
let execProtocolV2ScriptPath = "";
|
||||
let execMissingIdScriptPath = "";
|
||||
let execInheritedErrorScriptPath = "";
|
||||
let execProviderErrorScriptPath = "";
|
||||
let execUnsafeProviderErrorScriptPath = "";
|
||||
let execInvalidJsonScriptPath = "";
|
||||
let execFastExitScriptPath = "";
|
||||
|
||||
@@ -175,6 +177,26 @@ describe("secret ref resolver", () => {
|
||||
0o700,
|
||||
);
|
||||
|
||||
execProviderErrorScriptPath = path.join(sharedExecDir, "resolver-error.sh");
|
||||
await writeSecureFile(
|
||||
execProviderErrorScriptPath,
|
||||
[
|
||||
"#!/bin/sh",
|
||||
'printf \'{"protocolVersion":1,"values":{},"errors":{"openai/api-key":{"code":"NOT_FOUND","message":"provider-private-detail-7f3c"}}}\'',
|
||||
].join("\n"),
|
||||
0o700,
|
||||
);
|
||||
|
||||
execUnsafeProviderErrorScriptPath = path.join(sharedExecDir, "resolver-unsafe-error.sh");
|
||||
await writeSecureFile(
|
||||
execUnsafeProviderErrorScriptPath,
|
||||
[
|
||||
"#!/bin/sh",
|
||||
'printf \'{"protocolVersion":1,"values":{},"errors":{"openai/api-key":{"code":"PROVIDERPRIVATEDETAIL9C2E"}}}\'',
|
||||
].join("\n"),
|
||||
0o700,
|
||||
);
|
||||
|
||||
execInvalidJsonScriptPath = path.join(sharedExecDir, "resolver-invalid-json.sh");
|
||||
await writeSecureFile(
|
||||
execInvalidJsonScriptPath,
|
||||
@@ -239,6 +261,30 @@ describe("secret ref resolver", () => {
|
||||
expect(value).toBe("value:openai/api-key");
|
||||
});
|
||||
|
||||
itPosix("surfaces bounded exec error codes without provider-supplied detail", async () => {
|
||||
const error = await resolveExecSecret(execProviderErrorScriptPath).catch(
|
||||
(caught: unknown) => caught,
|
||||
);
|
||||
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect((error as Error).message).toBe(
|
||||
'Exec provider "execmain" failed for id "openai/api-key" (NOT_FOUND).',
|
||||
);
|
||||
expect((error as Error).message).not.toContain("provider-private-detail-7f3c");
|
||||
});
|
||||
|
||||
itPosix("suppresses exec error codes outside the bounded format", async () => {
|
||||
const error = await resolveExecSecret(execUnsafeProviderErrorScriptPath).catch(
|
||||
(caught: unknown) => caught,
|
||||
);
|
||||
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect((error as Error).message).toBe(
|
||||
'Exec provider "execmain" failed for id "openai/api-key".',
|
||||
);
|
||||
expect((error as Error).message).not.toContain("PROVIDERPRIVATEDETAIL9C2E");
|
||||
});
|
||||
|
||||
itPosix("clamps oversized exec provider timeouts", async () => {
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
|
||||
|
||||
@@ -57,6 +57,8 @@ const DEFAULT_FILE_MAX_BYTES = 1024 * 1024;
|
||||
const DEFAULT_FILE_TIMEOUT_MS = 5_000;
|
||||
const DEFAULT_EXEC_TIMEOUT_MS = 5_000;
|
||||
const DEFAULT_EXEC_MAX_OUTPUT_BYTES = 1024 * 1024;
|
||||
// Exec diagnostics cross CLI, RPC, and log boundaries; surface only canonical safe codes.
|
||||
const SAFE_EXEC_ERROR_CODES = new Set(["AMBIGUOUS_DUPLICATE_KEY", "NOT_FOUND"]);
|
||||
const WINDOWS_ABS_PATH_PATTERN = /^[A-Za-z]:[\\/]/;
|
||||
const WINDOWS_UNC_PATH_PATTERN = /^\\\\[^\\]+\\[^\\]+/;
|
||||
|
||||
@@ -669,19 +671,13 @@ function parseExecValues(params: {
|
||||
for (const id of params.ids) {
|
||||
if (responseErrors && Object.hasOwn(responseErrors, id)) {
|
||||
const entry = responseErrors[id];
|
||||
if (isRecord(entry) && typeof entry.message === "string" && entry.message.trim()) {
|
||||
throw refResolutionError({
|
||||
source: "exec",
|
||||
provider: params.providerName,
|
||||
refId: id,
|
||||
message: `Exec provider "${params.providerName}" failed for id "${id}" (${entry.message.trim()}).`,
|
||||
});
|
||||
}
|
||||
const code = isRecord(entry) && typeof entry.code === "string" ? entry.code : null;
|
||||
const safeCode = code && SAFE_EXEC_ERROR_CODES.has(code) ? code : null;
|
||||
throw refResolutionError({
|
||||
source: "exec",
|
||||
provider: params.providerName,
|
||||
refId: id,
|
||||
message: `Exec provider "${params.providerName}" failed for id "${id}".`,
|
||||
message: `Exec provider "${params.providerName}" failed for id "${id}"${safeCode ? ` (${safeCode})` : ""}.`,
|
||||
});
|
||||
}
|
||||
if (!Object.hasOwn(responseValues, id)) {
|
||||
|
||||
@@ -56,4 +56,42 @@ describe("openclaw-bws-resolver", () => {
|
||||
errors: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("returns bounded error codes for missing and ambiguous keys", () => {
|
||||
const dir = makeTempDir();
|
||||
const fakeBwsPath = path.join(dir, "bws");
|
||||
writeFileSync(
|
||||
fakeBwsPath,
|
||||
[
|
||||
"#!/usr/bin/env node",
|
||||
"process.stdout.write(JSON.stringify([",
|
||||
' { key: "duplicate", value: "first" },',
|
||||
' { key: "duplicate", value: "second" },',
|
||||
"]));",
|
||||
].join("\n"),
|
||||
{ mode: 0o755 },
|
||||
);
|
||||
chmodSync(fakeBwsPath, 0o755);
|
||||
|
||||
const result = spawnSync(process.execPath, [resolverPath], {
|
||||
encoding: "utf8",
|
||||
env: {
|
||||
BWS_ACCESS_TOKEN: "test-token",
|
||||
BWS_BIN: fakeBwsPath,
|
||||
PATH: process.env.PATH ?? "",
|
||||
},
|
||||
input: JSON.stringify({ protocolVersion: 1, ids: ["missing", "duplicate"] }),
|
||||
});
|
||||
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.stderr).toBe("");
|
||||
expect(JSON.parse(result.stdout)).toEqual({
|
||||
protocolVersion: 1,
|
||||
values: {},
|
||||
errors: {
|
||||
missing: { code: "NOT_FOUND" },
|
||||
duplicate: { code: "AMBIGUOUS_DUPLICATE_KEY" },
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user