mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-03 05:10:23 +00:00
fix(exec): resolve remote approval regressions (#58792)
* fix(exec): restore remote approval policy defaults * fix(exec): handle headless cron approval conflicts * fix(exec): make allow-always durable * fix(exec): persist exact-command shell trust * fix(doctor): match host exec fallback * fix(exec): preserve blocked and inline approval state * Doctor: surface allow-always ask bypass * Doctor: match effective exec policy * Exec: match node durable command text * Exec: tighten durable approval security * Exec: restore owner approver fallback * Config: refresh Slack approval metadata --------- Co-authored-by: scoootscooob <zhentongfan@gmail.com>
This commit is contained in:
@@ -59,7 +59,7 @@ export const slackChannelConfigUiHints = {
|
||||
},
|
||||
"execApprovals.approvers": {
|
||||
label: "Slack Exec Approval Approvers",
|
||||
help: "Slack user IDs allowed to approve exec requests for this workspace account. Use Slack user IDs or user targets such as `U123`, `user:U123`, or `<@U123>`. If you leave this unset, OpenClaw falls back to owner IDs inferred from channels.slack.allowFrom, channels.slack.dm.allowFrom, and defaultTo when possible.",
|
||||
help: "Slack user IDs allowed to approve exec requests for this workspace account. Use Slack user IDs or user targets such as `U123`, `user:U123`, or `<@U123>`. If you leave this unset, OpenClaw falls back to commands.ownerAllowFrom when possible.",
|
||||
},
|
||||
"execApprovals.agentFilter": {
|
||||
label: "Slack Exec Approval Agent Filter",
|
||||
|
||||
@@ -29,19 +29,27 @@ function buildConfig(
|
||||
}
|
||||
|
||||
describe("slack exec approvals", () => {
|
||||
it("requires enablement and an explicit or inferred approver", () => {
|
||||
it("requires enablement and explicit or owner approvers", () => {
|
||||
expect(isSlackExecApprovalClientEnabled({ cfg: buildConfig() })).toBe(false);
|
||||
expect(isSlackExecApprovalClientEnabled({ cfg: buildConfig({ enabled: true }) })).toBe(false);
|
||||
expect(
|
||||
isSlackExecApprovalClientEnabled({
|
||||
cfg: buildConfig({ enabled: true }, { allowFrom: ["U123"] }),
|
||||
}),
|
||||
).toBe(true);
|
||||
).toBe(false);
|
||||
expect(
|
||||
isSlackExecApprovalClientEnabled({
|
||||
cfg: buildConfig({ enabled: true, approvers: ["U123"] }),
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isSlackExecApprovalClientEnabled({
|
||||
cfg: {
|
||||
...buildConfig({ enabled: true }),
|
||||
commands: { ownerAllowFrom: ["slack:U123OWNER"] },
|
||||
} as OpenClawConfig,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("prefers explicit approvers when configured", () => {
|
||||
@@ -55,7 +63,7 @@ describe("slack exec approvals", () => {
|
||||
expect(isSlackExecApprovalApprover({ cfg, senderId: "U123" })).toBe(false);
|
||||
});
|
||||
|
||||
it("infers approvers from allowFrom, dm.allowFrom, and DM defaultTo", () => {
|
||||
it("does not infer approvers from allowFrom or DM default routes", () => {
|
||||
const cfg = buildConfig(
|
||||
{ enabled: true },
|
||||
{
|
||||
@@ -65,19 +73,18 @@ describe("slack exec approvals", () => {
|
||||
},
|
||||
);
|
||||
|
||||
expect(getSlackExecApprovalApprovers({ cfg })).toEqual(["U123", "U456", "U789"]);
|
||||
expect(isSlackExecApprovalApprover({ cfg, senderId: "U789" })).toBe(true);
|
||||
expect(getSlackExecApprovalApprovers({ cfg })).toEqual([]);
|
||||
expect(isSlackExecApprovalApprover({ cfg, senderId: "U789" })).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores non-user default targets when inferring approvers", () => {
|
||||
const cfg = buildConfig(
|
||||
{ enabled: true },
|
||||
{
|
||||
defaultTo: "channel:C123",
|
||||
},
|
||||
);
|
||||
it("falls back to commands.ownerAllowFrom for exec approvers", () => {
|
||||
const cfg = {
|
||||
...buildConfig({ enabled: true }),
|
||||
commands: { ownerAllowFrom: ["slack:U123", "user:U456", "<@U789>"] },
|
||||
} as OpenClawConfig;
|
||||
|
||||
expect(getSlackExecApprovalApprovers({ cfg })).toEqual([]);
|
||||
expect(getSlackExecApprovalApprovers({ cfg })).toEqual(["U123", "U456", "U789"]);
|
||||
expect(isSlackExecApprovalApprover({ cfg, senderId: "U456" })).toBe(true);
|
||||
});
|
||||
|
||||
it("defaults target to dm", () => {
|
||||
|
||||
@@ -28,6 +28,17 @@ export function normalizeSlackApproverId(value: string | number): string | undef
|
||||
return /^[UW][A-Z0-9]+$/i.test(trimmed) ? trimmed : undefined;
|
||||
}
|
||||
|
||||
function resolveSlackOwnerApprovers(cfg: OpenClawConfig): string[] {
|
||||
const ownerAllowFrom = cfg.commands?.ownerAllowFrom;
|
||||
if (!Array.isArray(ownerAllowFrom) || ownerAllowFrom.length === 0) {
|
||||
return [];
|
||||
}
|
||||
return resolveApprovalApprovers({
|
||||
explicit: ownerAllowFrom,
|
||||
normalizeApprover: normalizeSlackApproverId,
|
||||
});
|
||||
}
|
||||
|
||||
export function shouldHandleSlackExecApprovalRequest(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId?: string | null;
|
||||
@@ -61,14 +72,11 @@ export function getSlackExecApprovalApprovers(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId?: string | null;
|
||||
}): string[] {
|
||||
const account = resolveSlackAccount(params).config;
|
||||
return resolveApprovalApprovers({
|
||||
explicit: account.execApprovals?.approvers,
|
||||
allowFrom: account.allowFrom,
|
||||
extraAllowFrom: account.dm?.allowFrom,
|
||||
defaultTo: account.defaultTo,
|
||||
explicit:
|
||||
resolveSlackAccount(params).config.execApprovals?.approvers ??
|
||||
resolveSlackOwnerApprovers(params.cfg),
|
||||
normalizeApprover: normalizeSlackApproverId,
|
||||
normalizeDefaultTo: normalizeSlackApproverId,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -10,13 +10,18 @@ vi.mock("../send.js", () => ({
|
||||
|
||||
let SlackExecApprovalHandler: typeof import("./exec-approvals.js").SlackExecApprovalHandler;
|
||||
|
||||
function buildConfig(target: "dm" | "channel" | "both" = "dm"): OpenClawConfig {
|
||||
function buildConfig(
|
||||
target: "dm" | "channel" | "both" = "dm",
|
||||
slackOverrides?: Partial<NonNullable<NonNullable<OpenClawConfig["channels"]>["slack"]>>,
|
||||
): OpenClawConfig {
|
||||
const configuredExecApprovals = slackOverrides?.execApprovals;
|
||||
return {
|
||||
channels: {
|
||||
slack: {
|
||||
botToken: "xoxb-test",
|
||||
appToken: "xapp-test",
|
||||
execApprovals: {
|
||||
...slackOverrides,
|
||||
execApprovals: configuredExecApprovals ?? {
|
||||
enabled: true,
|
||||
approvers: ["U123APPROVER"],
|
||||
target,
|
||||
@@ -159,4 +164,38 @@ describe("SlackExecApprovalHandler", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not treat allowFrom senders as approvers", async () => {
|
||||
const app = buildApp();
|
||||
const cfg = buildConfig("dm", {
|
||||
allowFrom: ["U123APPROVER"],
|
||||
execApprovals: { enabled: true, target: "dm" },
|
||||
});
|
||||
const handler = new SlackExecApprovalHandler({
|
||||
app,
|
||||
accountId: "default",
|
||||
config: cfg.channels!.slack!.execApprovals!,
|
||||
cfg,
|
||||
});
|
||||
|
||||
expect(handler.shouldHandle(buildRequest())).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts commands.ownerAllowFrom as exec approver fallback", async () => {
|
||||
const app = buildApp();
|
||||
const cfg = {
|
||||
...buildConfig("dm", {
|
||||
execApprovals: { enabled: true, target: "dm" },
|
||||
}),
|
||||
commands: { ownerAllowFrom: ["slack:U123APPROVER"] },
|
||||
} as OpenClawConfig;
|
||||
const handler = new SlackExecApprovalHandler({
|
||||
app,
|
||||
accountId: "default",
|
||||
config: cfg.channels!.slack!.execApprovals!,
|
||||
cfg,
|
||||
});
|
||||
|
||||
expect(handler.shouldHandle(buildRequest())).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,7 +14,11 @@ import {
|
||||
} from "openclaw/plugin-sdk/infra-runtime";
|
||||
import { logError } from "openclaw/plugin-sdk/text-runtime";
|
||||
import { slackNativeApprovalAdapter } from "../approval-native.js";
|
||||
import { getSlackExecApprovalApprovers, normalizeSlackApproverId } from "../exec-approvals.js";
|
||||
import {
|
||||
getSlackExecApprovalApprovers,
|
||||
normalizeSlackApproverId,
|
||||
shouldHandleSlackExecApprovalRequest,
|
||||
} from "../exec-approvals.js";
|
||||
import { resolveSlackReplyBlocks } from "../reply-blocks.js";
|
||||
import { sendMessageSlack } from "../send.js";
|
||||
|
||||
@@ -240,20 +244,11 @@ export class SlackExecApprovalHandler {
|
||||
}
|
||||
|
||||
shouldHandle(request: ExecApprovalRequest): boolean {
|
||||
if (!this.opts.config.enabled) {
|
||||
return false;
|
||||
}
|
||||
if ((this.opts.config.approvers?.length ?? 0) === 0) {
|
||||
return false;
|
||||
}
|
||||
return (
|
||||
slackNativeApprovalAdapter.native?.describeDeliveryCapabilities({
|
||||
cfg: this.opts.cfg,
|
||||
accountId: this.opts.accountId,
|
||||
approvalKind: "exec",
|
||||
request,
|
||||
}).enabled === true
|
||||
);
|
||||
return shouldHandleSlackExecApprovalRequest({
|
||||
cfg: this.opts.cfg,
|
||||
accountId: this.opts.accountId,
|
||||
request,
|
||||
});
|
||||
}
|
||||
|
||||
async start(): Promise<void> {
|
||||
|
||||
Reference in New Issue
Block a user