ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-12 21:53:00 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(webhooks): keep routes cold when SecretRefs are unavailable (#109715)

Browse Source 
* fix(webhooks): isolate unresolved route secrets

* chore(plugin-sdk): refresh API baseline

* docs(secrets): refresh credential surface
This commit is contained in:
Peter Steinberger
2026-07-16 23:56:59 -07:00
committed by GitHub
parent a838c68dbe
commit c13925b387
15 changed files with 202 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plugins/manifest.md
+8 -7
View File
@@ -660,8 +660,9 @@ Use `configContracts` for manifest-owned config behavior that generic core helpe
"bundledDefaultEnabled": false,
"paths": [
{
"path": "apiKey",
"expected": "string"
"path": "routes.*.secret",
"expected": "string",
"ownerKind": "route"
}
]
}
@@ -674,7 +675,7 @@ Use `configContracts` for manifest-owned config behavior that generic core helpe
| `compatibilityMigrationPaths` | No | `string[]` | Root-relative config paths that indicate this plugin's setup-time compatibility migrations might apply. Lets generic runtime config reads skip every plugin setup surface when the config never references the plugin. |
| `compatibilityRuntimePaths` | No | `string[]` | Root-relative compatibility paths this plugin can service during runtime before plugin code fully activates. Use this for legacy surfaces that should narrow bundled candidate sets without importing every compatible plugin runtime. |
| `dangerousFlags` | No | `object[]` | Config literals that `openclaw doctor` should flag as insecure or dangerous when enabled. See below. |
| `secretInputs` | No | `object` | Config paths under `plugins.entries.<id>.config` that the SecretRef migration/audit target registry should treat as secret-shaped strings. See below. |
| `secretInputs` | No | `object` | Config paths under `plugins.entries.<id>.config` for SecretRef migration, audit, startup materialization, and optional runtime owner isolation. See below. |
Each `dangerousFlags` entry supports:
@@ -685,10 +686,10 @@ Each `dangerousFlags` entry supports:
`secretInputs` supports:
| Field | Required | Type | What it means |
| ----------------------- | -------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `bundledDefaultEnabled` | No | `boolean` | Override bundled-plugin default enablement when deciding whether this SecretRef surface is active. Use this when the plugin is bundled but the surface should stay inactive until explicitly enabled in config. |
| `paths` | Yes | `object[]` | Secret-shaped config paths, each with `path` (dot-separated, relative to `plugins.entries.<id>.config`, supports `*` wildcards) and optional `expected` (currently only `"string"`). |
| Field | Required | Type | What it means |
| ----------------------- | -------- | ---------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `bundledDefaultEnabled` | No | `boolean` | Override bundled-plugin default enablement when deciding whether this SecretRef surface is active. Use this when the plugin is bundled but the surface should stay inactive until explicitly enabled in config. |
| `paths` | Yes | `object[]` | Secret-shaped config paths, each with `path` (dot-separated, relative to `plugins.entries.<id>.config`, supports `*` wildcards), optional `expected` (currently only `"string"`), and optional `ownerKind` (currently only `"route"`). A declared owner isolates only that exact matched path when resolution fails; its owner id is the full config path. |
## mediaUnderstandingProviderMetadata reference
docs/plugins/webhooks.md
+6 -6
View File
@@ -58,12 +58,12 @@ Route fields:
`secret` accepts a plain string or a SecretRef: `{ source: "env" | "file" | "exec", provider: "default", id: "..." }`.
Every configured route registers at startup regardless of whether its secret
currently resolves. An unresolvable secret does not disable or skip the
route - requests to it fail authentication (`401`) until the secret can be
resolved. SecretRef values are re-resolved on every request, so rotating the
underlying secret (env var, file, or exec output) takes effect without a
Gateway restart.
SecretRefs resolve into the Gateway's startup config snapshot. When one route's
secret cannot resolve, the Gateway keeps running and that exact route stays
registered but cold: requests receive a generic authentication failure (`401`).
Other routes remain available. Fix the SecretRef source, then reload or restart
the Gateway to activate the new snapshot. SecretRef values are never resolved
on the public request path.
## Security model
docs/reference/secretref-credential-surface.md
+1
View File
@@ -61,6 +61,7 @@ The lists below are generated from the source target registry and checked agains
- `plugins.entries.voice-call.config.streaming.providers.*.apiKey`
- `plugins.entries.voice-call.config.tts.providers.*.apiKey`
- `plugins.entries.voice-call.config.twilio.authToken`
- `plugins.entries.webhooks.config.routes.*.secret`
- `tools.web.search.*.apiKey`
- `tools.web.search.apiKey`
- `gateway.auth.password`
docs/reference/secretref-user-supplied-credentials-matrix.json
+7
View File
@@ -687,6 +687,13 @@
"secretShape": "secret_input",
"optIn": true
},
{
"id": "plugins.entries.webhooks.config.routes.*.secret",
"configFile": "openclaw.json",
"path": "plugins.entries.webhooks.config.routes.*.secret",
"secretShape": "secret_input",
"optIn": true
},
{
"id": "plugins.entries.xai.config.webSearch.apiKey",
"configFile": "openclaw.json",
extensions/webhooks/index.ts
-1
View File
@@ -25,7 +25,6 @@ function registerWebhookRoutes(api: OpenClawPluginApi): void {
routeId: route.routeId,
path: route.path,
secretInput: route.secret,
secretConfigPath: `plugins.entries.webhooks.routes.${route.routeId}.secret`,
defaultControllerId: route.controllerId,
taskFlow,
};
extensions/webhooks/openclaw.plugin.json
+11
View File
@@ -5,6 +5,17 @@
},
"name": "Webhooks",
"description": "Authenticated inbound webhooks that bind external automation to OpenClaw TaskFlows.",
"configContracts": {
"secretInputs": {
"paths": [
{
"path": "routes.*.secret",
"expected": "string",
"ownerKind": "route"
}
]
}
},
"configSchema": {
"type": "object",
"additionalProperties": false,
extensions/webhooks/runtime-api.ts
-1
View File
@@ -12,5 +12,4 @@ export {
WEBHOOK_RATE_LIMIT_DEFAULTS,
type WebhookInFlightLimiter,
} from "openclaw/plugin-sdk/webhook-ingress";
export { resolveConfiguredSecretInputString } from "openclaw/plugin-sdk/secret-input-runtime";
export type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
extensions/webhooks/src/http.test.ts
+21 -33
View File
@@ -21,24 +21,6 @@ function createManagedFlow(
return flow;
}
const hoisted = vi.hoisted(() => {
const resolveConfiguredSecretInputStringMock = vi.fn();
return {
resolveConfiguredSecretInputStringMock,
};
});
vi.mock("../runtime-api.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../runtime-api.js")>();
hoisted.resolveConfiguredSecretInputStringMock.mockImplementation(
actual.resolveConfiguredSecretInputString,
);
return {
...actual,
resolveConfiguredSecretInputString: hoisted.resolveConfiguredSecretInputStringMock,
};
});
type MockIncomingMessage = IncomingMessage & {
destroyed?: boolean;
destroy: () => MockIncomingMessage;
@@ -74,19 +56,17 @@ function createJsonRequest(params: {
return req;
}
function createHandler(): {
function createHandler(secret = "shared-secret"): {
handler: ReturnType<typeof createTaskFlowWebhookRequestHandler>;
target: TaskFlowWebhookTarget;
secret: string;
} {
const runtime = createRuntimeTaskFlow();
nextSessionId += 1;
const secret = "shared-secret";
const target: TaskFlowWebhookTarget = {
routeId: "zapier",
path: "/plugins/webhooks/zapier",
secretInput: secret,
secretConfigPath: "plugins.entries.webhooks.routes.zapier.secret",
defaultControllerId: "webhooks/zapier",
taskFlow: runtime.bindSession({
sessionKey: `agent:main:webhook-test-${String(nextSessionId)}`,
@@ -153,10 +133,9 @@ describe("createTaskFlowWebhookRequestHandler", () => {
expect(res.statusCode).toBe(401);
expect(res.body).toBe("unauthorized");
expect(target.taskFlow.list()).toStrictEqual([]);
expect(hoisted.resolveConfiguredSecretInputStringMock).not.toHaveBeenCalled();
});
it("re-resolves SecretRef-backed secrets across requests", async () => {
it("keeps an unresolved SecretRef-backed route cold", async () => {
const runtime = createRuntimeTaskFlow();
const target: TaskFlowWebhookTarget = {
routeId: "cached",
@@ -166,16 +145,11 @@ describe("createTaskFlowWebhookRequestHandler", () => {
provider: "default",
id: "OPENCLAW_WEBHOOK_SECRET",
},
secretConfigPath: "plugins.entries.webhooks.routes.cached.secret",
defaultControllerId: "webhooks/cached",
taskFlow: runtime.bindSession({
sessionKey: "agent:main:webhook-cached",
}),
};
hoisted.resolveConfiguredSecretInputStringMock
.mockResolvedValueOnce({ value: "shared-secret" })
.mockResolvedValueOnce({ value: "rotated-secret" })
.mockResolvedValueOnce({ value: "rotated-secret" });
const handler = createHandlerWithTarget(target);
const first = await dispatchJsonRequest({
@@ -203,11 +177,25 @@ describe("createTaskFlowWebhookRequestHandler", () => {
},
});
expect(first.statusCode).toBe(200);
expect(second.statusCode).toBe(401);
expect(second.body).toBe("unauthorized");
expect(third.statusCode).toBe(200);
expect(hoisted.resolveConfiguredSecretInputStringMock).toHaveBeenCalledTimes(3);
expect([first, second, third].map((response) => response.statusCode)).toEqual([401, 401, 401]);
expect([first, second, third].map((response) => response.body)).toEqual([
"unauthorized",
"unauthorized",
"unauthorized",
]);
});
it("accepts a resolved secret that has env-template syntax", async () => {
const { handler, target } = createHandler("${MATERIALIZED_SECRET}");
const response = await dispatchJsonRequest({
handler,
path: target.path,
secret: "${MATERIALIZED_SECRET}",
body: { action: "list_flows" },
});
expect(response.statusCode).toBe(200);
});
it("creates flows through the bound session and scrubs owner metadata from responses", async () => {
extensions/webhooks/src/http.ts
+7 -22
View File
@@ -9,8 +9,7 @@ import {
createWebhookInFlightLimiter,
readJsonWebhookBodyOrReject,
resolveRequestClientIp,
resolveConfiguredSecretInputString,
resolveWebhookTargetWithAuthOrReject,
resolveWebhookTargetWithAuthOrRejectSync,
withResolvedWebhookRequestPipeline,
WEBHOOK_IN_FLIGHT_DEFAULTS,
WEBHOOK_RATE_LIMIT_DEFAULTS,
@@ -156,7 +155,6 @@ export type TaskFlowWebhookTarget = {
routeId: string;
path: string;
secretInput: WebhookSecretInput;
secretConfigPath: string;
defaultControllerId: string;
taskFlow: BoundTaskFlowRuntime;
};
@@ -703,21 +701,6 @@ export function createTaskFlowWebhookRequestHandler(params: {
maxInFlightPerKey: WEBHOOK_IN_FLIGHT_DEFAULTS.maxInFlightPerKey,
maxTrackedKeys: WEBHOOK_IN_FLIGHT_DEFAULTS.maxTrackedKeys,
});
const resolveTargetSecret = async (
target: TaskFlowWebhookTarget,
): Promise<string | undefined> => {
if (typeof target.secretInput === "string") {
return target.secretInput;
}
const resolved = await resolveConfiguredSecretInputString({
config: params.cfg,
env: process.env,
value: target.secretInput,
path: target.secretConfigPath,
});
return resolved.value;
};
return async (req: IncomingMessage, res: ServerResponse): Promise<boolean> => {
return await withResolvedWebhookRequestPipeline({
req,
@@ -740,15 +723,17 @@ export function createTaskFlowWebhookRequestHandler(params: {
inFlightLimiter,
handle: async ({ targets }) => {
const presentedSecret = extractSharedSecret(req);
const target = await resolveWebhookTargetWithAuthOrReject({
const target = resolveWebhookTargetWithAuthOrRejectSync({
targets,
res,
isMatch: async (candidate) => {
isMatch: (candidate) => {
if (presentedSecret.length === 0) {
return false;
}
const resolvedSecret = await resolveTargetSecret(candidate);
return Boolean(resolvedSecret && safeEqualSecret(resolvedSecret, presentedSecret));
return (
typeof candidate.secretInput === "string" &&
safeEqualSecret(candidate.secretInput, presentedSecret)
);
},
});
if (!target) {
src/plugins/manifest-registry.test.ts
+2 -2
View File
@@ -2555,7 +2555,7 @@ describe("loadPluginManifestRegistry", () => {
dangerousFlags: [{ path: "permissionMode", equals: "approve-all" }],
secretInputs: {
bundledDefaultEnabled: false,
paths: [{ path: "mcpServers.*.env.*", expected: "string" }],
paths: [{ path: "mcpServers.*.env.*", expected: "string", ownerKind: "route" }],
},
},
});
@@ -2572,7 +2572,7 @@ describe("loadPluginManifestRegistry", () => {
dangerousFlags: [{ path: "permissionMode", equals: "approve-all" }],
secretInputs: {
bundledDefaultEnabled: false,
paths: [{ path: "mcpServers.*.env.*", expected: "string" }],
paths: [{ path: "mcpServers.*.env.*", expected: "string", ownerKind: "route" }],
},
});
});
src/plugins/manifest.ts
+4
View File
@@ -259,6 +259,8 @@ type PluginManifestSecretInputPath = {
path: string;
/** Expected resolved type for SecretRef materialization. */
expected?: "string";
/** Runtime owner kind used to isolate this surface when resolution fails. */
ownerKind?: "route";
};
type PluginManifestSecretInputContracts = {
@@ -972,9 +974,11 @@ function normalizeManifestSecretInputPaths(
continue;
}
const expected = entry.expected === "string" ? entry.expected : undefined;
const ownerKind = entry.ownerKind === "route" ? entry.ownerKind : undefined;
normalized.push({
path: pathLocal,
...(expected ? { expected } : {}),
...(ownerKind ? { ownerKind } : {}),
});
}
return normalized.length > 0 ? normalized : undefined;
src/secrets/runtime-config-collectors-plugins.bundled.test.ts
+44
View File
@@ -13,6 +13,50 @@ function envRef(id: string) {
}
describe("collectPluginConfigAssignments bundled plugin manifests", () => {
it("assigns each webhooks route SecretRef to its exact runtime owner", () => {
expect(
findBundledPluginMetadataById("webhooks", {
includeChannelConfigs: false,
includeSyntheticChannelConfigs: false,
})?.manifest.configContracts?.secretInputs?.paths,
).toEqual([{ path: "routes.*.secret", expected: "string", ownerKind: "route" }]);
const config = {
plugins: {
entries: {
webhooks: {
enabled: true,
config: {
routes: {
zapier: {
sessionKey: "agent:main:main",
secret: envRef("WEBHOOK_SECRET"),
},
},
},
},
},
},
} as OpenClawConfig;
const context = createResolverContext({ sourceConfig: config, env: {} });
collectPluginConfigAssignments({
config,
defaults: undefined,
context,
loadablePluginOrigins: new Map([["webhooks", "bundled"]]),
});
expect(context.assignments).toMatchObject([
{
path: "plugins.entries.webhooks.config.routes.zapier.secret",
ownerKind: "route",
ownerId: "plugins.entries.webhooks.config.routes.zapier.secret",
requiredForGateway: false,
disposition: "isolate",
},
]);
});
it("collects Codex app-server SecretRefs from bundled manifest contracts", () => {
expect(
findBundledPluginMetadataById("codex", {
src/secrets/runtime-config-collectors-plugins.ts
+14 -4
View File
@@ -10,7 +10,7 @@ import { normalizePluginsConfig, resolveEnableState } from "../plugins/config-st
import type { PluginOrigin } from "../plugins/plugin-origin.types.js";
import { parseConfigPathArrayIndex } from "../shared/path-array-index.js";
import {
collectSecretInputAssignment,
collectRuntimeSecretInputAssignment,
type ResolverContext,
type SecretDefaults,
} from "./runtime-shared.js";
@@ -133,7 +133,7 @@ export function collectPluginConfigAssignments(params: {
function collectConfiguredPluginSecretAssignments(params: {
pluginId: string;
pluginConfig: Record<string, unknown>;
secretPaths: ReadonlyArray<{ path: string; expected?: "string" }>;
secretPaths: ReadonlyArray<{ path: string; expected?: "string"; ownerKind?: "route" }>;
active: boolean;
inactiveReason: string;
defaults: SecretDefaults | undefined;
@@ -153,8 +153,8 @@ function collectConfiguredPluginSecretAssignments(params: {
// SecretInput allows both explicit objects and inline env-template refs
// like `${MCP_API_KEY}`. Non-ref strings remain untouched because
// collectSecretInputAssignment ignores them.
collectSecretInputAssignment({
// collectRuntimeSecretInputAssignment ignores them.
collectRuntimeSecretInputAssignment({
value: match.value,
path: fullPath,
expected: secretPath.expected ?? "string",
@@ -162,6 +162,16 @@ function collectConfiguredPluginSecretAssignments(params: {
context: params.context,
active: params.active,
inactiveReason: `plugin "${params.pluginId}": ${params.inactiveReason}`,
...(secretPath.ownerKind
? {
owner: {
ownerKind: secretPath.ownerKind,
ownerId: fullPath,
requiredForGateway: false,
disposition: "isolate" as const,
},
}
: {}),
apply: createPluginConfigAssignmentApply(params.pluginConfig, match.path),
});
}
src/secrets/runtime-owner-assignments.ts
+5 -2
View File
@@ -159,8 +159,11 @@ export async function resolveAndApplySecretAssignments(params: {
for (const assignments of pendingOwners) {
const failureReason = failedOwners.get(assignments);
if (failureReason) {
// Leave explicit SecretRefs in runtime config. Applying another credential source here
// would silently route this owner through env/profile fallback after its declared ref failed.
// Canonicalize shorthand refs so runtime consumers can distinguish an unavailable ref
// from a successfully resolved literal that happens to look like `${ENV_VAR}`.
for (const assignment of assignments) {
assignment.apply({ ...assignment.ref });
}
const degradedOwner = createDegradedOwner(assignments, failureReason);
degradedOwners.push(degradedOwner);
warnDegradedSecretOwner(params.context, degradedOwner);
src/secrets/runtime.test.ts
+72
View File
@@ -9,6 +9,7 @@ import { asConfig, setupSecretsRuntimeSnapshotTestHooks } from "./runtime.test-s
const EMPTY_LOADABLE_PLUGIN_ORIGINS = new Map();
const BUNDLED_CODEX_PLUGIN_ORIGINS = new Map([["codex", "bundled" as const]]);
const BUNDLED_WEBHOOKS_PLUGIN_ORIGINS = new Map([["webhooks", "bundled" as const]]);
const { prepareSecretsRuntimeSnapshot } = setupSecretsRuntimeSnapshotTestHooks();
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
@@ -41,6 +42,77 @@ function expectWarning(
}
describe("secrets runtime snapshot", () => {
it("isolates one webhooks route while resolving its sibling snapshot", async () => {
const missingRef = {
source: "env",
provider: "default",
id: "MISSING_WEBHOOK_SECRET",
} as const;
const snapshot = await prepareSecretsRuntimeSnapshot({
config: asConfig({
plugins: {
entries: {
webhooks: {
enabled: true,
config: {
routes: {
healthy: {
sessionKey: "agent:main:main",
secret: {
source: "env",
provider: "default",
id: "HEALTHY_WEBHOOK_SECRET",
},
},
cold: {
sessionKey: "agent:main:main",
secret: missingRef,
},
inlineCold: {
sessionKey: "agent:main:main",
secret: "${MISSING_INLINE_WEBHOOK_SECRET}",
},
},
},
},
},
},
}),
env: { HEALTHY_WEBHOOK_SECRET: "healthy-secret" },
includeAuthStoreRefs: false,
allowUnavailableSecretOwners: true,
loadablePluginOrigins: BUNDLED_WEBHOOKS_PLUGIN_ORIGINS,
});
const routes = snapshot.config.plugins?.entries?.webhooks?.config?.routes as Record<
string,
{ secret?: unknown }
>;
expect(routes.healthy?.secret).toBe("healthy-secret");
expect(routes.cold?.secret).toEqual(missingRef);
expect(routes.inlineCold?.secret).toEqual({
source: "env",
provider: "default",
id: "MISSING_INLINE_WEBHOOK_SECRET",
});
expect(snapshot.degradedOwners).toMatchObject([
{
ownerKind: "route",
ownerId: "plugins.entries.webhooks.config.routes.cold.secret",
state: "unavailable",
paths: ["plugins.entries.webhooks.config.routes.cold.secret"],
reason: "secret reference was not found",
},
{
ownerKind: "route",
ownerId: "plugins.entries.webhooks.config.routes.inlineCold.secret",
state: "unavailable",
paths: ["plugins.entries.webhooks.config.routes.inlineCold.secret"],
reason: "secret reference was not found",
},
]);
});
it("registers every resolved value for exact redaction", async () => {
const secret = "runtime-registration-secret";
await prepareSecretsRuntimeSnapshot({