feat(session): add dmScope for multi-user DM isolation
Co-authored-by: Alphonse-arianee <Alphonse-arianee@users.noreply.github.com>
This commit is contained in:
committed by
Peter Steinberger
parent
e6364d031d
commit
ca9688b5cc
@@ -34,6 +34,8 @@ export async function noteSecurityWarnings(cfg: ClawdbotConfig) {
|
||||
.map((v) => v.trim())
|
||||
.filter(Boolean);
|
||||
const allowCount = Array.from(new Set([...normalizedCfg, ...normalizedStore])).length;
|
||||
const dmScope = cfg.session?.dmScope ?? "main";
|
||||
const isMultiUserDm = hasWildcard || allowCount > 1;
|
||||
|
||||
if (dmPolicy === "open") {
|
||||
const allowFromPath = `${params.allowFromPath}allowFrom`;
|
||||
@@ -43,7 +45,6 @@ export async function noteSecurityWarnings(cfg: ClawdbotConfig) {
|
||||
`- ${params.label} DMs: config invalid — "open" requires ${allowFromPath} to include "*".`,
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (dmPolicy === "disabled") {
|
||||
@@ -51,12 +52,18 @@ export async function noteSecurityWarnings(cfg: ClawdbotConfig) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (allowCount === 0) {
|
||||
if (dmPolicy !== "open" && allowCount === 0) {
|
||||
warnings.push(
|
||||
`- ${params.label} DMs: locked (${policyPath}="${dmPolicy}") with no allowlist; unknown senders will be blocked / get a pairing code.`,
|
||||
);
|
||||
warnings.push(` ${params.approveHint}`);
|
||||
}
|
||||
|
||||
if (dmScope === "main" && isMultiUserDm) {
|
||||
warnings.push(
|
||||
`- ${params.label} DMs: multiple senders share the main session; set session.dmScope="per-channel-peer" to isolate sessions.`,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
for (const plugin of listChannelPlugins()) {
|
||||
|
||||
@@ -167,6 +167,7 @@ async function noteChannelPrimer(
|
||||
"DM security: default is pairing; unknown DMs get a pairing code.",
|
||||
"Approve with: clawdbot pairing approve <channel> <code>",
|
||||
'Public DMs require dmPolicy="open" + allowFrom=["*"].',
|
||||
'Multi-user DMs: set session.dmScope="per-channel-peer" to isolate sessions.',
|
||||
`Docs: ${formatDocsLink("/start/pairing", "start/pairing")}`,
|
||||
"",
|
||||
...channelLines,
|
||||
@@ -212,6 +213,7 @@ async function maybeConfigureDmPolicies(params: {
|
||||
"Default: pairing (unknown DMs get a pairing code).",
|
||||
`Approve: clawdbot pairing approve ${policy.channel} <code>`,
|
||||
`Public DMs: ${policy.policyKey}="open" + ${policy.allowFromKey} includes "*".`,
|
||||
'Multi-user DMs: set session.dmScope="per-channel-peer" to isolate sessions.',
|
||||
`Docs: ${formatDocsLink("/start/pairing", "start/pairing")}`,
|
||||
].join("\n"),
|
||||
`${policy.label} DM access`,
|
||||
|
||||
@@ -167,6 +167,7 @@ const FIELD_LABELS: Record<string, string> = {
|
||||
"commands.useAccessGroups": "Use Access Groups",
|
||||
"ui.seamColor": "Accent Color",
|
||||
"browser.controlUrl": "Browser Control URL",
|
||||
"session.dmScope": "DM Session Scope",
|
||||
"session.agentToAgent.maxPingPongTurns": "Agent-to-Agent Ping-Pong Turns",
|
||||
"messages.ackReaction": "Ack Reaction Emoji",
|
||||
"messages.ackReactionScope": "Ack Reaction Scope",
|
||||
@@ -311,6 +312,8 @@ const FIELD_HELP: Record<string, string> = {
|
||||
"commands.debug": "Allow /debug chat command for runtime-only overrides (default: false).",
|
||||
"commands.restart": "Allow /restart and gateway restart tool actions (default: false).",
|
||||
"commands.useAccessGroups": "Enforce access-group allowlists/policies for commands.",
|
||||
"session.dmScope":
|
||||
'DM session scoping: "main" keeps continuity; "per-peer" or "per-channel-peer" isolates DM history (recommended for shared inboxes).',
|
||||
"channels.telegram.configWrites":
|
||||
"Allow Telegram to write config in response to channel events/commands (default: true).",
|
||||
"channels.slack.configWrites":
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
export type ReplyMode = "text" | "command";
|
||||
export type TypingMode = "never" | "instant" | "thinking" | "message";
|
||||
export type SessionScope = "per-sender" | "global";
|
||||
export type DmScope = "main" | "per-peer" | "per-channel-peer";
|
||||
export type ReplyToMode = "off" | "first" | "all";
|
||||
export type GroupPolicy = "open" | "disabled" | "allowlist";
|
||||
export type DmPolicy = "pairing" | "allowlist" | "open" | "disabled";
|
||||
@@ -54,6 +55,8 @@ export type SessionSendPolicyConfig = {
|
||||
|
||||
export type SessionConfig = {
|
||||
scope?: SessionScope;
|
||||
/** DM session scoping (default: "main"). */
|
||||
dmScope?: DmScope;
|
||||
resetTriggers?: string[];
|
||||
idleMinutes?: number;
|
||||
heartbeatIdleMinutes?: number;
|
||||
|
||||
@@ -10,6 +10,11 @@ import {
|
||||
export const SessionSchema = z
|
||||
.object({
|
||||
scope: z.union([z.literal("per-sender"), z.literal("global")]).optional(),
|
||||
dmScope: z.union([
|
||||
z.literal("main"),
|
||||
z.literal("per-peer"),
|
||||
z.literal("per-channel-peer"),
|
||||
]).optional(),
|
||||
resetTriggers: z.array(z.string()).optional(),
|
||||
idleMinutes: z.number().int().positive().optional(),
|
||||
heartbeatIdleMinutes: z.number().int().positive().optional(),
|
||||
|
||||
@@ -18,6 +18,32 @@ describe("resolveAgentRoute", () => {
|
||||
expect(route.matchedBy).toBe("default");
|
||||
});
|
||||
|
||||
test("dmScope=per-peer isolates DM sessions by sender id", () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
session: { dmScope: "per-peer" },
|
||||
};
|
||||
const route = resolveAgentRoute({
|
||||
cfg,
|
||||
channel: "whatsapp",
|
||||
accountId: null,
|
||||
peer: { kind: "dm", id: "+15551234567" },
|
||||
});
|
||||
expect(route.sessionKey).toBe("agent:main:dm:+15551234567");
|
||||
});
|
||||
|
||||
test("dmScope=per-channel-peer isolates DM sessions per channel and sender", () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
session: { dmScope: "per-channel-peer" },
|
||||
};
|
||||
const route = resolveAgentRoute({
|
||||
cfg,
|
||||
channel: "whatsapp",
|
||||
accountId: null,
|
||||
peer: { kind: "dm", id: "+15551234567" },
|
||||
});
|
||||
expect(route.sessionKey).toBe("agent:main:whatsapp:dm:+15551234567");
|
||||
});
|
||||
|
||||
test("peer binding wins over account binding", () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
bindings: [
|
||||
|
||||
@@ -68,6 +68,8 @@ export function buildAgentSessionKey(params: {
|
||||
agentId: string;
|
||||
channel: string;
|
||||
peer?: RoutePeer | null;
|
||||
/** DM session scope. */
|
||||
dmScope?: "main" | "per-peer" | "per-channel-peer";
|
||||
}): string {
|
||||
const channel = normalizeToken(params.channel) || "unknown";
|
||||
const peer = params.peer;
|
||||
@@ -77,6 +79,7 @@ export function buildAgentSessionKey(params: {
|
||||
channel,
|
||||
peerKind: peer?.kind ?? "dm",
|
||||
peerId: peer ? normalizeId(peer.id) || "unknown" : null,
|
||||
dmScope: params.dmScope,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -149,6 +152,8 @@ export function resolveAgentRoute(input: ResolveAgentRouteInput): ResolvedAgentR
|
||||
return matchesAccountId(binding.match?.accountId, accountId);
|
||||
});
|
||||
|
||||
const dmScope = input.cfg.session?.dmScope ?? "main";
|
||||
|
||||
const choose = (agentId: string, matchedBy: ResolvedAgentRoute["matchedBy"]) => {
|
||||
const resolvedAgentId = pickFirstExistingAgentId(input.cfg, agentId);
|
||||
return {
|
||||
@@ -159,6 +164,7 @@ export function resolveAgentRoute(input: ResolveAgentRouteInput): ResolvedAgentR
|
||||
agentId: resolvedAgentId,
|
||||
channel,
|
||||
peer,
|
||||
dmScope,
|
||||
}),
|
||||
mainSessionKey: buildAgentMainSessionKey({
|
||||
agentId: resolvedAgentId,
|
||||
|
||||
@@ -88,9 +88,20 @@ export function buildAgentPeerSessionKey(params: {
|
||||
channel: string;
|
||||
peerKind?: "dm" | "group" | "channel" | null;
|
||||
peerId?: string | null;
|
||||
/** DM session scope. */
|
||||
dmScope?: "main" | "per-peer" | "per-channel-peer";
|
||||
}): string {
|
||||
const peerKind = params.peerKind ?? "dm";
|
||||
if (peerKind === "dm") {
|
||||
const dmScope = params.dmScope ?? "main";
|
||||
const peerId = (params.peerId ?? "").trim();
|
||||
if (dmScope === "per-channel-peer" && peerId) {
|
||||
const channel = (params.channel ?? "").trim().toLowerCase() || "unknown";
|
||||
return `agent:${normalizeAgentId(params.agentId)}:${channel}:dm:${peerId}`;
|
||||
}
|
||||
if (dmScope === "per-peer" && peerId) {
|
||||
return `agent:${normalizeAgentId(params.agentId)}:dm:${peerId}`;
|
||||
}
|
||||
return buildAgentMainSessionKey({
|
||||
agentId: params.agentId,
|
||||
mainKey: params.mainKey,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import type { ClawdbotConfig } from "../config/config.js";
|
||||
import type { ChannelPlugin } from "../channels/plugins/types.js";
|
||||
import { runSecurityAudit } from "./audit.js";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
@@ -173,6 +174,54 @@ describe("security audit", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("warns when multiple DM senders share the main session", async () => {
|
||||
const cfg: ClawdbotConfig = { session: { dmScope: "main" } };
|
||||
const plugins: ChannelPlugin[] = [
|
||||
{
|
||||
id: "whatsapp",
|
||||
meta: {
|
||||
id: "whatsapp",
|
||||
label: "WhatsApp",
|
||||
selectionLabel: "WhatsApp",
|
||||
docsPath: "/channels/whatsapp",
|
||||
blurb: "Test",
|
||||
},
|
||||
capabilities: { chatTypes: ["direct"] },
|
||||
config: {
|
||||
listAccountIds: () => ["default"],
|
||||
resolveAccount: () => ({}),
|
||||
isEnabled: () => true,
|
||||
isConfigured: () => true,
|
||||
},
|
||||
security: {
|
||||
resolveDmPolicy: () => ({
|
||||
policy: "allowlist",
|
||||
allowFrom: ["user-a", "user-b"],
|
||||
policyPath: "channels.whatsapp.dmPolicy",
|
||||
allowFromPath: "channels.whatsapp.",
|
||||
approveHint: "approve",
|
||||
}),
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
const res = await runSecurityAudit({
|
||||
config: cfg,
|
||||
includeFilesystem: false,
|
||||
includeChannelSecurity: true,
|
||||
plugins,
|
||||
});
|
||||
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "channels.whatsapp.dm.scope_main_multiuser",
|
||||
severity: "warn",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("adds a warning when deep probe fails", async () => {
|
||||
const cfg: ClawdbotConfig = { gateway: { mode: "local" } };
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
collectSyncedFolderFindings,
|
||||
readConfigSnapshotForAudit,
|
||||
} from "./audit-extra.js";
|
||||
import { readChannelAllowFromStore } from "../pairing/pairing-store.js";
|
||||
import {
|
||||
formatOctal,
|
||||
isGroupReadable,
|
||||
@@ -386,10 +387,25 @@ async function collectChannelSecurityFindings(params: {
|
||||
allowFrom?: Array<string | number> | null;
|
||||
policyPath?: string;
|
||||
allowFromPath: string;
|
||||
normalizeEntry?: (raw: string) => string;
|
||||
}) => {
|
||||
const policyPath = input.policyPath ?? `${input.allowFromPath}policy`;
|
||||
const configAllowFrom = normalizeAllowFromList(input.allowFrom);
|
||||
const hasWildcard = configAllowFrom.includes("*");
|
||||
const dmScope = params.cfg.session?.dmScope ?? "main";
|
||||
const storeAllowFrom = await readChannelAllowFromStore(input.provider).catch(() => []);
|
||||
const normalizeEntry = input.normalizeEntry ?? ((value: string) => value);
|
||||
const normalizedCfg = configAllowFrom
|
||||
.filter((value) => value !== "*")
|
||||
.map((value) => normalizeEntry(value))
|
||||
.map((value) => value.trim())
|
||||
.filter(Boolean);
|
||||
const normalizedStore = storeAllowFrom
|
||||
.map((value) => normalizeEntry(value))
|
||||
.map((value) => value.trim())
|
||||
.filter(Boolean);
|
||||
const allowCount = Array.from(new Set([...normalizedCfg, ...normalizedStore])).length;
|
||||
const isMultiUserDm = hasWildcard || allowCount > 1;
|
||||
|
||||
if (input.dmPolicy === "open") {
|
||||
const allowFromKey = `${input.allowFromPath}allowFrom`;
|
||||
@@ -408,7 +424,6 @@ async function collectChannelSecurityFindings(params: {
|
||||
detail: `"open" requires ${allowFromKey} to include "*".`,
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (input.dmPolicy === "disabled") {
|
||||
@@ -418,6 +433,18 @@ async function collectChannelSecurityFindings(params: {
|
||||
title: `${input.label} DMs are disabled`,
|
||||
detail: `${policyPath}="disabled" ignores inbound DMs.`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (dmScope === "main" && isMultiUserDm) {
|
||||
findings.push({
|
||||
checkId: `channels.${input.provider}.dm.scope_main_multiuser`,
|
||||
severity: "warn",
|
||||
title: `${input.label} DMs share the main session`,
|
||||
detail:
|
||||
"Multiple DM senders currently share the main session, which can leak context across users.",
|
||||
remediation: 'Set session.dmScope="per-channel-peer" to isolate DM sessions per sender.',
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
@@ -450,6 +477,7 @@ async function collectChannelSecurityFindings(params: {
|
||||
allowFrom: dmPolicy.allowFrom,
|
||||
policyPath: dmPolicy.policyPath,
|
||||
allowFromPath: dmPolicy.allowFromPath,
|
||||
normalizeEntry: dmPolicy.normalizeEntry,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -58,6 +58,7 @@ export async function maybeBroadcastMessage(params: {
|
||||
kind: params.msg.chatType === "group" ? "group" : "dm",
|
||||
id: params.peerId,
|
||||
},
|
||||
dmScope: params.cfg.session?.dmScope,
|
||||
}),
|
||||
mainSessionKey: buildAgentMainSessionKey({
|
||||
agentId: normalizedAgentId,
|
||||
|
||||
Reference in New Issue
Block a user