fix(imessage): keep DM pairing-store identities out of group allowlist auth
This commit is contained in:
parent
e6f67d5f31
commit
872079d42f
@ -31,6 +31,29 @@ function getConfig(): TestConfig {
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe("monitorIMessageProvider", () => {
|
describe("monitorIMessageProvider", () => {
|
||||||
|
it("ignores malformed rpc message payloads", async () => {
|
||||||
|
const run = monitorIMessageProvider();
|
||||||
|
await waitForSubscribe();
|
||||||
|
|
||||||
|
notificationHandler?.({
|
||||||
|
method: "message",
|
||||||
|
params: {
|
||||||
|
message: {
|
||||||
|
id: 1,
|
||||||
|
sender: { nested: "not-a-string" },
|
||||||
|
text: "hello",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
closeResolve?.();
|
||||||
|
await run;
|
||||||
|
|
||||||
|
expect(replyMock).not.toHaveBeenCalled();
|
||||||
|
expect(sendMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("skips group messages without a mention by default", async () => {
|
it("skips group messages without a mention by default", async () => {
|
||||||
const run = monitorIMessageProvider();
|
const run = monitorIMessageProvider();
|
||||||
await waitForSubscribe();
|
await waitForSubscribe();
|
||||||
@ -364,6 +387,127 @@ describe("monitorIMessageProvider", () => {
|
|||||||
await run;
|
await run;
|
||||||
|
|
||||||
expect(replyMock).not.toHaveBeenCalled();
|
expect(replyMock).not.toHaveBeenCalled();
|
||||||
|
expect(sendMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not allow group sender from pairing store when groupPolicy is allowlist", async () => {
|
||||||
|
config = {
|
||||||
|
...config,
|
||||||
|
channels: {
|
||||||
|
...config.channels,
|
||||||
|
imessage: {
|
||||||
|
...config.channels?.imessage,
|
||||||
|
dmPolicy: "pairing",
|
||||||
|
allowFrom: [],
|
||||||
|
groupPolicy: "allowlist",
|
||||||
|
groupAllowFrom: [],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
readAllowFromStoreMock.mockResolvedValue(["+15550003333"]);
|
||||||
|
const run = monitorIMessageProvider();
|
||||||
|
await waitForSubscribe();
|
||||||
|
|
||||||
|
notificationHandler?.({
|
||||||
|
method: "message",
|
||||||
|
params: {
|
||||||
|
message: {
|
||||||
|
id: 30,
|
||||||
|
chat_id: 909,
|
||||||
|
sender: "+15550003333",
|
||||||
|
is_from_me: false,
|
||||||
|
text: "@openclaw hi from paired sender",
|
||||||
|
is_group: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
closeResolve?.();
|
||||||
|
await run;
|
||||||
|
|
||||||
|
expect(replyMock).not.toHaveBeenCalled();
|
||||||
|
expect(sendMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not allow sender from pairing store when groupAllowFrom is restricted to a different chat_id", async () => {
|
||||||
|
config = {
|
||||||
|
...config,
|
||||||
|
channels: {
|
||||||
|
...config.channels,
|
||||||
|
imessage: {
|
||||||
|
...config.channels?.imessage,
|
||||||
|
dmPolicy: "pairing",
|
||||||
|
allowFrom: [],
|
||||||
|
groupPolicy: "allowlist",
|
||||||
|
groupAllowFrom: ["chat_id:101"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
readAllowFromStoreMock.mockResolvedValue(["+15550003333"]);
|
||||||
|
const run = monitorIMessageProvider();
|
||||||
|
await waitForSubscribe();
|
||||||
|
|
||||||
|
notificationHandler?.({
|
||||||
|
method: "message",
|
||||||
|
params: {
|
||||||
|
message: {
|
||||||
|
id: 31,
|
||||||
|
chat_id: 202,
|
||||||
|
sender: "+15550003333",
|
||||||
|
is_from_me: false,
|
||||||
|
text: "@openclaw hi from paired sender",
|
||||||
|
is_group: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
closeResolve?.();
|
||||||
|
await run;
|
||||||
|
|
||||||
|
expect(replyMock).not.toHaveBeenCalled();
|
||||||
|
expect(sendMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not authorize control command via pairing-store sender in non-allowlisted chat", async () => {
|
||||||
|
config = {
|
||||||
|
...config,
|
||||||
|
channels: {
|
||||||
|
...config.channels,
|
||||||
|
imessage: {
|
||||||
|
...config.channels?.imessage,
|
||||||
|
dmPolicy: "pairing",
|
||||||
|
allowFrom: [],
|
||||||
|
groupPolicy: "allowlist",
|
||||||
|
groupAllowFrom: ["chat_id:101"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
readAllowFromStoreMock.mockResolvedValue(["+15550003333"]);
|
||||||
|
const run = monitorIMessageProvider();
|
||||||
|
await waitForSubscribe();
|
||||||
|
|
||||||
|
notificationHandler?.({
|
||||||
|
method: "message",
|
||||||
|
params: {
|
||||||
|
message: {
|
||||||
|
id: 32,
|
||||||
|
chat_id: 202,
|
||||||
|
sender: "+15550003333",
|
||||||
|
is_from_me: false,
|
||||||
|
text: "/status",
|
||||||
|
is_group: true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await flush();
|
||||||
|
closeResolve?.();
|
||||||
|
await run;
|
||||||
|
|
||||||
|
expect(replyMock).not.toHaveBeenCalled();
|
||||||
|
expect(sendMock).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("blocks group messages when groupPolicy is disabled", async () => {
|
it("blocks group messages when groupPolicy is disabled", async () => {
|
||||||
|
|||||||
@ -111,6 +111,88 @@ function describeReplyContext(message: IMessagePayload): IMessageReplyContext |
|
|||||||
return { body, id, sender };
|
return { body, id, sender };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
|
return Boolean(value) && typeof value === "object" && !Array.isArray(value);
|
||||||
|
}
|
||||||
|
|
||||||
|
function isOptionalString(value: unknown): value is string | null | undefined {
|
||||||
|
return value === undefined || value === null || typeof value === "string";
|
||||||
|
}
|
||||||
|
|
||||||
|
function isOptionalStringOrNumber(value: unknown): value is string | number | null | undefined {
|
||||||
|
return (
|
||||||
|
value === undefined || value === null || typeof value === "string" || typeof value === "number"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function isOptionalNumber(value: unknown): value is number | null | undefined {
|
||||||
|
return value === undefined || value === null || typeof value === "number";
|
||||||
|
}
|
||||||
|
|
||||||
|
function isOptionalBoolean(value: unknown): value is boolean | null | undefined {
|
||||||
|
return value === undefined || value === null || typeof value === "boolean";
|
||||||
|
}
|
||||||
|
|
||||||
|
function isOptionalStringArray(value: unknown): value is string[] | null | undefined {
|
||||||
|
return (
|
||||||
|
value === undefined ||
|
||||||
|
value === null ||
|
||||||
|
(Array.isArray(value) && value.every((entry) => typeof entry === "string"))
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function isOptionalAttachments(value: unknown): value is IMessagePayload["attachments"] {
|
||||||
|
if (value === undefined || value === null) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (!Array.isArray(value)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return value.every((attachment) => {
|
||||||
|
if (!isRecord(attachment)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return (
|
||||||
|
isOptionalString(attachment.original_path) &&
|
||||||
|
isOptionalString(attachment.mime_type) &&
|
||||||
|
isOptionalBoolean(attachment.missing)
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseIMessageNotification(raw: unknown): IMessagePayload | null {
|
||||||
|
if (!isRecord(raw)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
const maybeMessage = raw.message;
|
||||||
|
if (!isRecord(maybeMessage)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
const message: IMessagePayload = maybeMessage;
|
||||||
|
if (
|
||||||
|
!isOptionalNumber(message.id) ||
|
||||||
|
!isOptionalNumber(message.chat_id) ||
|
||||||
|
!isOptionalString(message.sender) ||
|
||||||
|
!isOptionalBoolean(message.is_from_me) ||
|
||||||
|
!isOptionalString(message.text) ||
|
||||||
|
!isOptionalStringOrNumber(message.reply_to_id) ||
|
||||||
|
!isOptionalString(message.reply_to_text) ||
|
||||||
|
!isOptionalString(message.reply_to_sender) ||
|
||||||
|
!isOptionalString(message.created_at) ||
|
||||||
|
!isOptionalAttachments(message.attachments) ||
|
||||||
|
!isOptionalString(message.chat_identifier) ||
|
||||||
|
!isOptionalString(message.chat_guid) ||
|
||||||
|
!isOptionalString(message.chat_name) ||
|
||||||
|
!isOptionalStringArray(message.participants) ||
|
||||||
|
!isOptionalBoolean(message.is_group)
|
||||||
|
) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
return message;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Cache for recently sent messages, used for echo detection.
|
* Cache for recently sent messages, used for echo detection.
|
||||||
* Keys are scoped by conversation (accountId:target) so the same text in different chats is not conflated.
|
* Keys are scoped by conversation (accountId:target) so the same text in different chats is not conflated.
|
||||||
@ -294,7 +376,9 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P
|
|||||||
const effectiveDmAllowFrom = Array.from(new Set([...allowFrom, ...storeAllowFrom]))
|
const effectiveDmAllowFrom = Array.from(new Set([...allowFrom, ...storeAllowFrom]))
|
||||||
.map((v) => String(v).trim())
|
.map((v) => String(v).trim())
|
||||||
.filter(Boolean);
|
.filter(Boolean);
|
||||||
const effectiveGroupAllowFrom = Array.from(new Set([...groupAllowFrom, ...storeAllowFrom]))
|
// Keep DM pairing-store authorization scoped to DMs; group access must come
|
||||||
|
// from explicit group allowlist config.
|
||||||
|
const effectiveGroupAllowFrom = Array.from(new Set(groupAllowFrom))
|
||||||
.map((v) => String(v).trim())
|
.map((v) => String(v).trim())
|
||||||
.filter(Boolean);
|
.filter(Boolean);
|
||||||
|
|
||||||
@ -676,9 +760,9 @@ export async function monitorIMessageProvider(opts: MonitorIMessageOpts = {}): P
|
|||||||
}
|
}
|
||||||
|
|
||||||
const handleMessage = async (raw: unknown) => {
|
const handleMessage = async (raw: unknown) => {
|
||||||
const params = raw as { message?: IMessagePayload | null };
|
const message = parseIMessageNotification(raw);
|
||||||
const message = params?.message ?? null;
|
|
||||||
if (!message) {
|
if (!message) {
|
||||||
|
logVerbose("imessage: dropping malformed RPC message payload");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
await inboundDebouncer.enqueue({ message });
|
await inboundDebouncer.enqueue({ message });
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user