mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-08 19:12:22 +00:00
fix(slack): normalize approval user ids (#84671)
* fix(slack): normalize approval user ids * chore(openrouter): satisfy spread fallback lint * fix(ci): unblock status and secret-file checks
This commit is contained in:
@@ -23,6 +23,15 @@ describe("slackApprovalAuth", () => {
|
|||||||
}),
|
}),
|
||||||
).toEqual({ authorized: true });
|
).toEqual({ authorized: true });
|
||||||
|
|
||||||
|
expect(
|
||||||
|
slackApprovalAuth.authorizeActorAction({
|
||||||
|
cfg,
|
||||||
|
senderId: "u123owner",
|
||||||
|
action: "approve",
|
||||||
|
approvalKind: "plugin",
|
||||||
|
}),
|
||||||
|
).toEqual({ authorized: true });
|
||||||
|
|
||||||
expect(
|
expect(
|
||||||
slackApprovalAuth.authorizeActorAction({
|
slackApprovalAuth.authorizeActorAction({
|
||||||
cfg,
|
cfg,
|
||||||
@@ -32,6 +41,15 @@ describe("slackApprovalAuth", () => {
|
|||||||
}),
|
}),
|
||||||
).toEqual({ authorized: true });
|
).toEqual({ authorized: true });
|
||||||
|
|
||||||
|
expect(
|
||||||
|
slackApprovalAuth.authorizeActorAction({
|
||||||
|
cfg,
|
||||||
|
senderId: "u345default",
|
||||||
|
action: "approve",
|
||||||
|
approvalKind: "plugin",
|
||||||
|
}),
|
||||||
|
).toEqual({ authorized: true });
|
||||||
|
|
||||||
expect(
|
expect(
|
||||||
slackApprovalAuth.authorizeActorAction({
|
slackApprovalAuth.authorizeActorAction({
|
||||||
cfg,
|
cfg,
|
||||||
@@ -56,4 +74,26 @@ describe("slackApprovalAuth", () => {
|
|||||||
reason: "❌ You are not authorized to approve exec requests on Slack.",
|
reason: "❌ You are not authorized to approve exec requests on Slack.",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("canonicalizes configured plugin approver ids before matching uppercase senders", () => {
|
||||||
|
const cfg = {
|
||||||
|
channels: {
|
||||||
|
slack: {
|
||||||
|
allowFrom: ["slack:u123owner"],
|
||||||
|
defaultTo: "user:u345default",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
for (const senderId of ["U123OWNER", "U345DEFAULT"]) {
|
||||||
|
expect(
|
||||||
|
slackApprovalAuth.authorizeActorAction({
|
||||||
|
cfg,
|
||||||
|
senderId,
|
||||||
|
action: "approve",
|
||||||
|
approvalKind: "plugin",
|
||||||
|
}),
|
||||||
|
).toEqual({ authorized: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -69,9 +69,25 @@ describe("slack exec approvals", () => {
|
|||||||
|
|
||||||
expect(getSlackExecApprovalApprovers({ cfg })).toEqual(["U456"]);
|
expect(getSlackExecApprovalApprovers({ cfg })).toEqual(["U456"]);
|
||||||
expect(isSlackExecApprovalApprover({ cfg, senderId: "U456" })).toBe(true);
|
expect(isSlackExecApprovalApprover({ cfg, senderId: "U456" })).toBe(true);
|
||||||
|
expect(isSlackExecApprovalApprover({ cfg, senderId: "u456" })).toBe(true);
|
||||||
expect(isSlackExecApprovalApprover({ cfg, senderId: "U123" })).toBe(false);
|
expect(isSlackExecApprovalApprover({ cfg, senderId: "U123" })).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("canonicalizes configured exec approver ids before matching uppercase senders", () => {
|
||||||
|
const explicitCfg = buildConfig({ approvers: ["u456"] });
|
||||||
|
expect(getSlackExecApprovalApprovers({ cfg: explicitCfg })).toEqual(["U456"]);
|
||||||
|
expect(isSlackExecApprovalApprover({ cfg: explicitCfg, senderId: "U456" })).toBe(true);
|
||||||
|
|
||||||
|
const ownerFallbackCfg = {
|
||||||
|
...buildConfig({ enabled: true }),
|
||||||
|
commands: { ownerAllowFrom: ["slack:u123owner"] },
|
||||||
|
} as OpenClawConfig;
|
||||||
|
expect(getSlackExecApprovalApprovers({ cfg: ownerFallbackCfg })).toEqual(["U123OWNER"]);
|
||||||
|
expect(isSlackExecApprovalApprover({ cfg: ownerFallbackCfg, senderId: "U123OWNER" })).toBe(
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it("does not infer approvers from allowFrom or DM default routes", () => {
|
it("does not infer approvers from allowFrom or DM default routes", () => {
|
||||||
const cfg = buildConfig(
|
const cfg = buildConfig(
|
||||||
{ enabled: true },
|
{ enabled: true },
|
||||||
@@ -115,7 +131,7 @@ describe("slack exec approvals", () => {
|
|||||||
enabled: true,
|
enabled: true,
|
||||||
mode: "targets",
|
mode: "targets",
|
||||||
targets: [
|
targets: [
|
||||||
{ channel: "slack", to: "user:U123TARGET" },
|
{ channel: "slack", to: "user:u123target" },
|
||||||
{ channel: "slack", to: "channel:C123" },
|
{ channel: "slack", to: "channel:C123" },
|
||||||
],
|
],
|
||||||
},
|
},
|
||||||
@@ -123,8 +139,10 @@ describe("slack exec approvals", () => {
|
|||||||
} as OpenClawConfig;
|
} as OpenClawConfig;
|
||||||
|
|
||||||
expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "U123TARGET" })).toBe(true);
|
expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "U123TARGET" })).toBe(true);
|
||||||
|
expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "u123target" })).toBe(true);
|
||||||
expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "U999OTHER" })).toBe(false);
|
expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "U999OTHER" })).toBe(false);
|
||||||
expect(isSlackExecApprovalAuthorizedSender({ cfg, senderId: "U123TARGET" })).toBe(true);
|
expect(isSlackExecApprovalAuthorizedSender({ cfg, senderId: "U123TARGET" })).toBe(true);
|
||||||
|
expect(isSlackExecApprovalAuthorizedSender({ cfg, senderId: "u123target" })).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("keeps the local Slack approval prompt path active", () => {
|
it("keeps the local Slack approval prompt path active", () => {
|
||||||
@@ -154,7 +172,15 @@ describe("slack exec approvals", () => {
|
|||||||
|
|
||||||
it("normalizes wrapped sender ids", () => {
|
it("normalizes wrapped sender ids", () => {
|
||||||
expect(normalizeSlackApproverId("user:U123OWNER")).toBe("U123OWNER");
|
expect(normalizeSlackApproverId("user:U123OWNER")).toBe("U123OWNER");
|
||||||
|
expect(normalizeSlackApproverId("user:u123owner")).toBe("U123OWNER");
|
||||||
|
expect(normalizeSlackApproverId("slack:u123owner")).toBe("U123OWNER");
|
||||||
expect(normalizeSlackApproverId("<@U123OWNER>")).toBe("U123OWNER");
|
expect(normalizeSlackApproverId("<@U123OWNER>")).toBe("U123OWNER");
|
||||||
|
expect(normalizeSlackApproverId("<@u123owner>")).toBe("U123OWNER");
|
||||||
|
expect(normalizeSlackApproverId("u123owner")).toBe("U123OWNER");
|
||||||
|
expect(normalizeSlackApproverId("C123CHANNEL")).toBeUndefined();
|
||||||
|
expect(normalizeSlackApproverId("slack:C123CHANNEL")).toBeUndefined();
|
||||||
|
expect(normalizeSlackApproverId("user:C123CHANNEL")).toBeUndefined();
|
||||||
|
expect(normalizeSlackApproverId("<@C123CHANNEL>")).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("applies agent and session filters to request handling", () => {
|
it("applies agent and session filters to request handling", () => {
|
||||||
|
|||||||
@@ -8,6 +8,11 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
|
|||||||
import { normalizeStringifiedOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime";
|
import { normalizeStringifiedOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||||
import { resolveSlackAccount } from "./accounts.js";
|
import { resolveSlackAccount } from "./accounts.js";
|
||||||
|
|
||||||
|
function normalizeSlackUserLikeId(value: string): string | undefined {
|
||||||
|
const upper = value.toUpperCase();
|
||||||
|
return /^[UW][A-Z0-9]+$/.test(upper) ? upper : undefined;
|
||||||
|
}
|
||||||
|
|
||||||
export function normalizeSlackApproverId(value: string | number): string | undefined {
|
export function normalizeSlackApproverId(value: string | number): string | undefined {
|
||||||
const trimmed = normalizeStringifiedOptionalString(value);
|
const trimmed = normalizeStringifiedOptionalString(value);
|
||||||
if (!trimmed) {
|
if (!trimmed) {
|
||||||
@@ -15,13 +20,13 @@ export function normalizeSlackApproverId(value: string | number): string | undef
|
|||||||
}
|
}
|
||||||
const prefixed = trimmed.match(/^(?:slack|user):([A-Z0-9]+)$/i);
|
const prefixed = trimmed.match(/^(?:slack|user):([A-Z0-9]+)$/i);
|
||||||
if (prefixed?.[1]) {
|
if (prefixed?.[1]) {
|
||||||
return prefixed[1];
|
return normalizeSlackUserLikeId(prefixed[1]);
|
||||||
}
|
}
|
||||||
const mention = trimmed.match(/^<@([A-Z0-9]+)>$/i);
|
const mention = trimmed.match(/^<@([A-Z0-9]+)>$/i);
|
||||||
if (mention?.[1]) {
|
if (mention?.[1]) {
|
||||||
return mention[1];
|
return normalizeSlackUserLikeId(mention[1]);
|
||||||
}
|
}
|
||||||
return /^[UW][A-Z0-9]+$/i.test(trimmed) ? trimmed : undefined;
|
return normalizeSlackUserLikeId(trimmed);
|
||||||
}
|
}
|
||||||
|
|
||||||
function resolveSlackOwnerApprovers(cfg: OpenClawConfig): string[] {
|
function resolveSlackOwnerApprovers(cfg: OpenClawConfig): string[] {
|
||||||
|
|||||||
@@ -934,7 +934,20 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("resolves exec approvals from shared interactive Slack actions", async () => {
|
it("resolves exec approvals from shared interactive Slack actions", async () => {
|
||||||
const { ctx, app, getHandler } = createContext({ allowFrom: ["U999"] });
|
const { ctx, app, getHandler } = createContext({
|
||||||
|
allowFrom: ["U999"],
|
||||||
|
cfg: {
|
||||||
|
channels: {
|
||||||
|
slack: {
|
||||||
|
execApprovals: {
|
||||||
|
enabled: true,
|
||||||
|
approvers: ["u123"],
|
||||||
|
target: "both",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
registerSlackInteractionEvents({ ctx: ctx as never });
|
registerSlackInteractionEvents({ ctx: ctx as never });
|
||||||
|
|
||||||
const handler = getHandler();
|
const handler = getHandler();
|
||||||
@@ -997,7 +1010,7 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
slack: {
|
slack: {
|
||||||
accounts: {
|
accounts: {
|
||||||
default: {
|
default: {
|
||||||
allowFrom: ["U123OWNER"],
|
allowFrom: ["u123owner"],
|
||||||
execApprovals: {
|
execApprovals: {
|
||||||
enabled: true,
|
enabled: true,
|
||||||
approvers: ["U999EXEC"],
|
approvers: ["U999EXEC"],
|
||||||
@@ -1071,7 +1084,7 @@ describe("registerSlackInteractionEvents", () => {
|
|||||||
slack: {
|
slack: {
|
||||||
accounts: {
|
accounts: {
|
||||||
default: {
|
default: {
|
||||||
allowFrom: ["U123OWNER"],
|
allowFrom: ["u123owner"],
|
||||||
execApprovals: {
|
execApprovals: {
|
||||||
enabled: true,
|
enabled: true,
|
||||||
approvers: ["U999EXEC"],
|
approvers: ["U999EXEC"],
|
||||||
|
|||||||
@@ -1,5 +1,8 @@
|
|||||||
import "./fs-safe-defaults.js";
|
import "./fs-safe-defaults.js";
|
||||||
import { readSecretFileSync as readSecretFileSyncImpl } from "@openclaw/fs-safe/secret";
|
import {
|
||||||
|
readSecretFileSync as readSecretFileSyncImpl,
|
||||||
|
tryReadSecretFileSync as tryReadSecretFileSyncImpl,
|
||||||
|
} from "@openclaw/fs-safe/secret";
|
||||||
import { resolveUserPath } from "../utils.js";
|
import { resolveUserPath } from "../utils.js";
|
||||||
|
|
||||||
export {
|
export {
|
||||||
@@ -7,11 +10,22 @@ export {
|
|||||||
PRIVATE_SECRET_DIR_MODE,
|
PRIVATE_SECRET_DIR_MODE,
|
||||||
PRIVATE_SECRET_FILE_MODE,
|
PRIVATE_SECRET_FILE_MODE,
|
||||||
readSecretFileSync,
|
readSecretFileSync,
|
||||||
tryReadSecretFileSync,
|
|
||||||
type SecretFileReadOptions,
|
type SecretFileReadOptions,
|
||||||
} from "@openclaw/fs-safe/secret";
|
} from "@openclaw/fs-safe/secret";
|
||||||
export { writeSecretFileAtomic as writePrivateSecretFileAtomic } from "@openclaw/fs-safe/secret";
|
export { writeSecretFileAtomic as writePrivateSecretFileAtomic } from "@openclaw/fs-safe/secret";
|
||||||
|
|
||||||
|
export function tryReadSecretFileSync(
|
||||||
|
filePath: string | undefined,
|
||||||
|
label: string,
|
||||||
|
options: Parameters<typeof tryReadSecretFileSyncImpl>[2] = {},
|
||||||
|
): string | undefined {
|
||||||
|
try {
|
||||||
|
return tryReadSecretFileSyncImpl(filePath, label, options);
|
||||||
|
} catch {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export type SecretFileReadResult =
|
export type SecretFileReadResult =
|
||||||
| {
|
| {
|
||||||
ok: true;
|
ok: true;
|
||||||
|
|||||||
Reference in New Issue
Block a user