fix(signal): prevent sentTranscript sync messages from bypassing loop protection (#31093)
* fix(signal): prevent sentTranscript sync messages from bypassing loop protection Issue: #31084 On daemon restart, sentTranscript sync messages could bypass loop protection because the syncMessage check happened before the sender validation. This reorganizes the checks to: 1. First resolve the sender (phone or UUID) 2. Check if the message is from our own account (both phone and UUID) 3. Only skip sync messages from other sources after confirming not own account This ensures that sync messages from the own account are properly filtered to prevent self-reply loops, while still allowing messages synced from other devices to be processed. Added optional accountUuid config field for UUID-based account identification. * fix(signal): cover UUID-only own-message loop protection * build: regenerate host env security policy swift --------- Co-authored-by: Kevin Wang <kevin@example.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -6,6 +6,8 @@ export type SignalReactionLevel = "off" | "ack" | "minimal" | "extensive";
|
||||
export type SignalAccountConfig = CommonChannelMessagingConfig & {
|
||||
/** Optional explicit E.164 account for signal-cli. */
|
||||
account?: string;
|
||||
/** Optional account UUID for signal-cli (used for loop protection). */
|
||||
accountUuid?: string;
|
||||
/** Optional full base URL for signal-cli HTTP daemon. */
|
||||
httpUrl?: string;
|
||||
/** HTTP host for signal-cli daemon (default 127.0.0.1). */
|
||||
|
||||
@@ -423,6 +423,7 @@ export async function monitorSignalProvider(opts: MonitorSignalOpts = {}): Promi
|
||||
cfg,
|
||||
baseUrl,
|
||||
account,
|
||||
accountUuid: accountInfo.config.accountUuid,
|
||||
accountId: accountInfo.accountId,
|
||||
blockStreaming: accountInfo.config.blockStreaming,
|
||||
historyLimit,
|
||||
|
||||
@@ -172,4 +172,33 @@ describe("signal createSignalEventHandler inbound contract", () => {
|
||||
expect(capture.ctx).toBeTruthy();
|
||||
expect(capture.ctx?.CommandAuthorized).toBe(false);
|
||||
});
|
||||
|
||||
it("drops own UUID inbound messages when only accountUuid is configured", async () => {
|
||||
const ownUuid = "123e4567-e89b-12d3-a456-426614174000";
|
||||
const handler = createSignalEventHandler(
|
||||
createBaseSignalEventHandlerDeps({
|
||||
cfg: {
|
||||
messages: { inbound: { debounceMs: 0 } },
|
||||
channels: { signal: { dmPolicy: "open", allowFrom: ["*"], accountUuid: ownUuid } },
|
||||
},
|
||||
account: undefined,
|
||||
accountUuid: ownUuid,
|
||||
historyLimit: 0,
|
||||
}),
|
||||
);
|
||||
|
||||
await handler(
|
||||
createSignalReceiveEvent({
|
||||
sourceNumber: null,
|
||||
sourceUuid: ownUuid,
|
||||
dataMessage: {
|
||||
message: "self message",
|
||||
attachments: [],
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(capture.ctx).toBeUndefined();
|
||||
expect(dispatchInboundMessageMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -420,18 +420,28 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
||||
if (!envelope) {
|
||||
return;
|
||||
}
|
||||
if (envelope.syncMessage) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check for syncMessage (e.g., sentTranscript from other devices)
|
||||
// We need to check if it's from our own account to prevent self-reply loops
|
||||
const sender = resolveSignalSender(envelope);
|
||||
if (!sender) {
|
||||
return;
|
||||
}
|
||||
if (deps.account && sender.kind === "phone") {
|
||||
if (sender.e164 === normalizeE164(deps.account)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if the message is from our own account to prevent loop/self-reply
|
||||
// This handles both phone number and UUID based identification
|
||||
const normalizedAccount = deps.account ? normalizeE164(deps.account) : undefined;
|
||||
const isOwnMessage =
|
||||
(sender.kind === "phone" && normalizedAccount != null && sender.e164 === normalizedAccount) ||
|
||||
(sender.kind === "uuid" && deps.accountUuid != null && sender.raw === deps.accountUuid);
|
||||
if (isOwnMessage) {
|
||||
return;
|
||||
}
|
||||
|
||||
// For non-own sync messages (e.g., messages synced from other devices),
|
||||
// we could process them but for now we skip to be conservative
|
||||
if (envelope.syncMessage) {
|
||||
return;
|
||||
}
|
||||
|
||||
const dataMessage = envelope.dataMessage ?? envelope.editMessage?.dataMessage;
|
||||
|
||||
@@ -72,6 +72,7 @@ export type SignalEventHandlerDeps = {
|
||||
cfg: OpenClawConfig;
|
||||
baseUrl: string;
|
||||
account?: string;
|
||||
accountUuid?: string;
|
||||
accountId: string;
|
||||
blockStreaming?: boolean;
|
||||
historyLimit: number;
|
||||
|
||||
Reference in New Issue
Block a user