mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-09 20:44:32 +00:00
fix(signal): enforce auth before reaction notification enqueue
This commit is contained in:
@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
|
||||||
|
- Security/Signal: enforce DM/group authorization before reaction-only notification enqueue so unauthorized senders can no longer inject Signal reaction system events under `dmPolicy`/`groupPolicy`; reaction notifications now require channel access checks first. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
|
||||||
- Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting.
|
- Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting.
|
||||||
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
|
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
|
||||||
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
|
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
|
||||||
|
|||||||
@@ -378,6 +378,65 @@ describe("monitorSignalProvider tool results", () => {
|
|||||||
expect(events.some((text) => text.includes("Signal reaction added"))).toBe(true);
|
expect(events.some((text) => text.includes("Signal reaction added"))).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("blocks reaction notifications from unauthorized senders when dmPolicy is allowlist", async () => {
|
||||||
|
setReactionNotificationConfig("all", {
|
||||||
|
dmPolicy: "allowlist",
|
||||||
|
allowFrom: ["+15550007777"],
|
||||||
|
});
|
||||||
|
await receiveSingleEnvelope({
|
||||||
|
...makeBaseEnvelope(),
|
||||||
|
reactionMessage: {
|
||||||
|
emoji: "✅",
|
||||||
|
targetAuthor: "+15550002222",
|
||||||
|
targetSentTimestamp: 2,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const events = getDirectSignalEventsFor("+15550001111");
|
||||||
|
expect(events.some((text) => text.includes("Signal reaction added"))).toBe(false);
|
||||||
|
expect(sendMock).not.toHaveBeenCalled();
|
||||||
|
expect(upsertPairingRequestMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("blocks reaction notifications from unauthorized senders when dmPolicy is pairing", async () => {
|
||||||
|
setReactionNotificationConfig("own", {
|
||||||
|
dmPolicy: "pairing",
|
||||||
|
allowFrom: [],
|
||||||
|
account: "+15550009999",
|
||||||
|
});
|
||||||
|
await receiveSingleEnvelope({
|
||||||
|
...makeBaseEnvelope(),
|
||||||
|
reactionMessage: {
|
||||||
|
emoji: "✅",
|
||||||
|
targetAuthor: "+15550009999",
|
||||||
|
targetSentTimestamp: 2,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const events = getDirectSignalEventsFor("+15550001111");
|
||||||
|
expect(events.some((text) => text.includes("Signal reaction added"))).toBe(false);
|
||||||
|
expect(sendMock).not.toHaveBeenCalled();
|
||||||
|
expect(upsertPairingRequestMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows reaction notifications for allowlisted senders when dmPolicy is allowlist", async () => {
|
||||||
|
setReactionNotificationConfig("all", {
|
||||||
|
dmPolicy: "allowlist",
|
||||||
|
allowFrom: ["+15550001111"],
|
||||||
|
});
|
||||||
|
await receiveSingleEnvelope({
|
||||||
|
...makeBaseEnvelope(),
|
||||||
|
reactionMessage: {
|
||||||
|
emoji: "✅",
|
||||||
|
targetAuthor: "+15550002222",
|
||||||
|
targetSentTimestamp: 2,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const events = getDirectSignalEventsFor("+15550001111");
|
||||||
|
expect(events.some((text) => text.includes("Signal reaction added"))).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it("notifies on own reactions when target includes uuid + phone", async () => {
|
it("notifies on own reactions when target includes uuid + phone", async () => {
|
||||||
setReactionNotificationConfig("own", { account: "+15550002222" });
|
setReactionNotificationConfig("own", { account: "+15550002222" });
|
||||||
await receiveSingleEnvelope({
|
await receiveSingleEnvelope({
|
||||||
|
|||||||
@@ -36,6 +36,10 @@ import {
|
|||||||
upsertChannelPairingRequest,
|
upsertChannelPairingRequest,
|
||||||
} from "../../pairing/pairing-store.js";
|
} from "../../pairing/pairing-store.js";
|
||||||
import { resolveAgentRoute } from "../../routing/resolve-route.js";
|
import { resolveAgentRoute } from "../../routing/resolve-route.js";
|
||||||
|
import {
|
||||||
|
resolveDmGroupAccessDecision,
|
||||||
|
resolveEffectiveAllowFromLists,
|
||||||
|
} from "../../security/dm-policy-shared.js";
|
||||||
import { normalizeE164 } from "../../utils.js";
|
import { normalizeE164 } from "../../utils.js";
|
||||||
import {
|
import {
|
||||||
formatSignalPairingIdLine,
|
formatSignalPairingIdLine,
|
||||||
@@ -366,15 +370,45 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
|||||||
const quoteText = dataMessage?.quote?.text?.trim() ?? "";
|
const quoteText = dataMessage?.quote?.text?.trim() ?? "";
|
||||||
const hasBodyContent =
|
const hasBodyContent =
|
||||||
Boolean(messageText || quoteText) || Boolean(!reaction && dataMessage?.attachments?.length);
|
Boolean(messageText || quoteText) || Boolean(!reaction && dataMessage?.attachments?.length);
|
||||||
|
const senderDisplay = formatSignalSenderDisplay(sender);
|
||||||
|
const storeAllowFrom =
|
||||||
|
deps.dmPolicy === "allowlist"
|
||||||
|
? []
|
||||||
|
: await readChannelAllowFromStore("signal").catch(() => []);
|
||||||
|
const { effectiveAllowFrom: effectiveDmAllow } = resolveEffectiveAllowFromLists({
|
||||||
|
allowFrom: deps.allowFrom,
|
||||||
|
groupAllowFrom: deps.groupAllowFrom,
|
||||||
|
storeAllowFrom,
|
||||||
|
dmPolicy: deps.dmPolicy,
|
||||||
|
});
|
||||||
|
const effectiveGroupAllow = [...deps.groupAllowFrom, ...storeAllowFrom];
|
||||||
|
const resolveAccessDecision = (isGroup: boolean) =>
|
||||||
|
resolveDmGroupAccessDecision({
|
||||||
|
isGroup,
|
||||||
|
dmPolicy: deps.dmPolicy,
|
||||||
|
groupPolicy: deps.groupPolicy,
|
||||||
|
effectiveAllowFrom: effectiveDmAllow,
|
||||||
|
effectiveGroupAllowFrom: effectiveGroupAllow,
|
||||||
|
isSenderAllowed: (allowFrom) => isSignalSenderAllowed(sender, allowFrom),
|
||||||
|
});
|
||||||
|
const dmAccess = resolveAccessDecision(false);
|
||||||
|
const dmAllowed = dmAccess.decision === "allow";
|
||||||
|
|
||||||
if (reaction && !hasBodyContent) {
|
if (reaction && !hasBodyContent) {
|
||||||
if (reaction.isRemove) {
|
if (reaction.isRemove) {
|
||||||
return;
|
return;
|
||||||
} // Ignore reaction removals
|
} // Ignore reaction removals
|
||||||
const emojiLabel = reaction.emoji?.trim() || "emoji";
|
const emojiLabel = reaction.emoji?.trim() || "emoji";
|
||||||
const senderDisplay = formatSignalSenderDisplay(sender);
|
|
||||||
const senderName = envelope.sourceName ?? senderDisplay;
|
const senderName = envelope.sourceName ?? senderDisplay;
|
||||||
logVerbose(`signal reaction: ${emojiLabel} from ${senderName}`);
|
logVerbose(`signal reaction: ${emojiLabel} from ${senderName}`);
|
||||||
|
const groupId = reaction.groupInfo?.groupId ?? undefined;
|
||||||
|
const groupName = reaction.groupInfo?.groupName ?? undefined;
|
||||||
|
const isGroup = Boolean(groupId);
|
||||||
|
const reactionAccess = resolveAccessDecision(isGroup);
|
||||||
|
if (reactionAccess.decision !== "allow") {
|
||||||
|
logVerbose(`Blocked signal reaction sender ${senderDisplay} (${reactionAccess.reason})`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
const targets = deps.resolveSignalReactionTargets(reaction);
|
const targets = deps.resolveSignalReactionTargets(reaction);
|
||||||
const shouldNotify = deps.shouldEmitSignalReactionNotification({
|
const shouldNotify = deps.shouldEmitSignalReactionNotification({
|
||||||
mode: deps.reactionMode,
|
mode: deps.reactionMode,
|
||||||
@@ -387,9 +421,6 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const groupId = reaction.groupInfo?.groupId ?? undefined;
|
|
||||||
const groupName = reaction.groupInfo?.groupName ?? undefined;
|
|
||||||
const isGroup = Boolean(groupId);
|
|
||||||
const senderPeerId = resolveSignalPeerId(sender);
|
const senderPeerId = resolveSignalPeerId(sender);
|
||||||
const route = resolveAgentRoute({
|
const route = resolveAgentRoute({
|
||||||
cfg: deps.cfg,
|
cfg: deps.cfg,
|
||||||
@@ -430,7 +461,6 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const senderDisplay = formatSignalSenderDisplay(sender);
|
|
||||||
const senderRecipient = resolveSignalRecipient(sender);
|
const senderRecipient = resolveSignalRecipient(sender);
|
||||||
const senderPeerId = resolveSignalPeerId(sender);
|
const senderPeerId = resolveSignalPeerId(sender);
|
||||||
const senderAllowId = formatSignalSenderId(sender);
|
const senderAllowId = formatSignalSenderId(sender);
|
||||||
@@ -441,20 +471,15 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
|||||||
const groupId = dataMessage.groupInfo?.groupId ?? undefined;
|
const groupId = dataMessage.groupInfo?.groupId ?? undefined;
|
||||||
const groupName = dataMessage.groupInfo?.groupName ?? undefined;
|
const groupName = dataMessage.groupInfo?.groupName ?? undefined;
|
||||||
const isGroup = Boolean(groupId);
|
const isGroup = Boolean(groupId);
|
||||||
const storeAllowFrom =
|
|
||||||
deps.dmPolicy === "allowlist"
|
|
||||||
? []
|
|
||||||
: await readChannelAllowFromStore("signal").catch(() => []);
|
|
||||||
const effectiveDmAllow = [...deps.allowFrom, ...storeAllowFrom];
|
|
||||||
const effectiveGroupAllow = [...deps.groupAllowFrom, ...storeAllowFrom];
|
|
||||||
const dmAllowed =
|
|
||||||
deps.dmPolicy === "open" ? true : isSignalSenderAllowed(sender, effectiveDmAllow);
|
|
||||||
|
|
||||||
if (!isGroup) {
|
if (!isGroup) {
|
||||||
if (deps.dmPolicy === "disabled") {
|
if (dmAccess.decision === "block") {
|
||||||
|
if (deps.dmPolicy !== "disabled") {
|
||||||
|
logVerbose(`Blocked signal sender ${senderDisplay} (dmPolicy=${deps.dmPolicy})`);
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (!dmAllowed) {
|
if (dmAccess.decision === "pairing") {
|
||||||
if (deps.dmPolicy === "pairing") {
|
if (deps.dmPolicy === "pairing") {
|
||||||
const senderId = senderAllowId;
|
const senderId = senderAllowId;
|
||||||
const { code, created } = await upsertChannelPairingRequest({
|
const { code, created } = await upsertChannelPairingRequest({
|
||||||
@@ -483,23 +508,20 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
|||||||
logVerbose(`signal pairing reply failed for ${senderId}: ${String(err)}`);
|
logVerbose(`signal pairing reply failed for ${senderId}: ${String(err)}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
|
||||||
logVerbose(`Blocked signal sender ${senderDisplay} (dmPolicy=${deps.dmPolicy})`);
|
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (isGroup && deps.groupPolicy === "disabled") {
|
if (isGroup) {
|
||||||
logVerbose("Blocked signal group message (groupPolicy: disabled)");
|
const groupAccess = resolveAccessDecision(true);
|
||||||
return;
|
if (groupAccess.decision !== "allow") {
|
||||||
}
|
if (groupAccess.reason === "groupPolicy=disabled") {
|
||||||
if (isGroup && deps.groupPolicy === "allowlist") {
|
logVerbose("Blocked signal group message (groupPolicy: disabled)");
|
||||||
if (effectiveGroupAllow.length === 0) {
|
} else if (groupAccess.reason === "groupPolicy=allowlist (empty allowlist)") {
|
||||||
logVerbose("Blocked signal group message (groupPolicy: allowlist, no groupAllowFrom)");
|
logVerbose("Blocked signal group message (groupPolicy: allowlist, no groupAllowFrom)");
|
||||||
return;
|
} else {
|
||||||
}
|
logVerbose(`Blocked signal group sender ${senderDisplay} (not in groupAllowFrom)`);
|
||||||
if (!isSignalSenderAllowed(sender, effectiveGroupAllow)) {
|
}
|
||||||
logVerbose(`Blocked signal group sender ${senderDisplay} (not in groupAllowFrom)`);
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user