diff --git a/extensions/slack/src/approval-auth.test.ts b/extensions/slack/src/approval-auth.test.ts index 958f0fe8942..758f888b2dc 100644 --- a/extensions/slack/src/approval-auth.test.ts +++ b/extensions/slack/src/approval-auth.test.ts @@ -23,6 +23,15 @@ describe("slackApprovalAuth", () => { }), ).toEqual({ authorized: true }); + expect( + slackApprovalAuth.authorizeActorAction({ + cfg, + senderId: "u123owner", + action: "approve", + approvalKind: "plugin", + }), + ).toEqual({ authorized: true }); + expect( slackApprovalAuth.authorizeActorAction({ cfg, @@ -32,6 +41,15 @@ describe("slackApprovalAuth", () => { }), ).toEqual({ authorized: true }); + expect( + slackApprovalAuth.authorizeActorAction({ + cfg, + senderId: "u345default", + action: "approve", + approvalKind: "plugin", + }), + ).toEqual({ authorized: true }); + expect( slackApprovalAuth.authorizeActorAction({ cfg, @@ -56,4 +74,26 @@ describe("slackApprovalAuth", () => { 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 }); + } + }); }); diff --git a/extensions/slack/src/exec-approvals.test.ts b/extensions/slack/src/exec-approvals.test.ts index 8ab5be6b461..f1056eaa160 100644 --- a/extensions/slack/src/exec-approvals.test.ts +++ b/extensions/slack/src/exec-approvals.test.ts @@ -69,9 +69,25 @@ describe("slack exec approvals", () => { expect(getSlackExecApprovalApprovers({ cfg })).toEqual(["U456"]); expect(isSlackExecApprovalApprover({ cfg, senderId: "U456" })).toBe(true); + expect(isSlackExecApprovalApprover({ cfg, senderId: "u456" })).toBe(true); 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", () => { const cfg = buildConfig( { enabled: true }, @@ -115,7 +131,7 @@ describe("slack exec approvals", () => { enabled: true, mode: "targets", targets: [ - { channel: "slack", to: "user:U123TARGET" }, + { channel: "slack", to: "user:u123target" }, { channel: "slack", to: "channel:C123" }, ], }, @@ -123,8 +139,10 @@ describe("slack exec approvals", () => { } as OpenClawConfig; expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "U123TARGET" })).toBe(true); + expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "u123target" })).toBe(true); expect(isSlackExecApprovalTargetRecipient({ cfg, senderId: "U999OTHER" })).toBe(false); expect(isSlackExecApprovalAuthorizedSender({ cfg, senderId: "U123TARGET" })).toBe(true); + expect(isSlackExecApprovalAuthorizedSender({ cfg, senderId: "u123target" })).toBe(true); }); it("keeps the local Slack approval prompt path active", () => { @@ -154,7 +172,15 @@ describe("slack exec approvals", () => { it("normalizes wrapped sender ids", () => { 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("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", () => { diff --git a/extensions/slack/src/exec-approvals.ts b/extensions/slack/src/exec-approvals.ts index 3d5cbc559bc..8586290f057 100644 --- a/extensions/slack/src/exec-approvals.ts +++ b/extensions/slack/src/exec-approvals.ts @@ -8,6 +8,11 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { normalizeStringifiedOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; 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 { const trimmed = normalizeStringifiedOptionalString(value); if (!trimmed) { @@ -15,13 +20,13 @@ export function normalizeSlackApproverId(value: string | number): string | undef } const prefixed = trimmed.match(/^(?:slack|user):([A-Z0-9]+)$/i); if (prefixed?.[1]) { - return prefixed[1]; + return normalizeSlackUserLikeId(prefixed[1]); } const mention = trimmed.match(/^<@([A-Z0-9]+)>$/i); 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[] { diff --git a/extensions/slack/src/monitor/events/interactions.test.ts b/extensions/slack/src/monitor/events/interactions.test.ts index fd38f00eda2..42a9aafd0d8 100644 --- a/extensions/slack/src/monitor/events/interactions.test.ts +++ b/extensions/slack/src/monitor/events/interactions.test.ts @@ -934,7 +934,20 @@ describe("registerSlackInteractionEvents", () => { }); 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 }); const handler = getHandler(); @@ -997,7 +1010,7 @@ describe("registerSlackInteractionEvents", () => { slack: { accounts: { default: { - allowFrom: ["U123OWNER"], + allowFrom: ["u123owner"], execApprovals: { enabled: true, approvers: ["U999EXEC"], @@ -1071,7 +1084,7 @@ describe("registerSlackInteractionEvents", () => { slack: { accounts: { default: { - allowFrom: ["U123OWNER"], + allowFrom: ["u123owner"], execApprovals: { enabled: true, approvers: ["U999EXEC"], diff --git a/src/infra/secret-file.ts b/src/infra/secret-file.ts index 4cf28da5f95..97aeb67d53b 100644 --- a/src/infra/secret-file.ts +++ b/src/infra/secret-file.ts @@ -1,5 +1,8 @@ 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"; export { @@ -7,11 +10,22 @@ export { PRIVATE_SECRET_DIR_MODE, PRIVATE_SECRET_FILE_MODE, readSecretFileSync, - tryReadSecretFileSync, type SecretFileReadOptions, } from "@openclaw/fs-safe/secret"; export { writeSecretFileAtomic as writePrivateSecretFileAtomic } from "@openclaw/fs-safe/secret"; +export function tryReadSecretFileSync( + filePath: string | undefined, + label: string, + options: Parameters[2] = {}, +): string | undefined { + try { + return tryReadSecretFileSyncImpl(filePath, label, options); + } catch { + return undefined; + } +} + export type SecretFileReadResult = | { ok: true;