refactor(security): harden CommandAuthorized plumbing
This commit is contained in:
@@ -2,8 +2,8 @@ import type { IncomingMessage, ServerResponse } from "node:http";
|
||||
|
||||
import type { ResolvedZaloAccount } from "./accounts.js";
|
||||
import {
|
||||
hasInlineCommandTokens,
|
||||
isControlCommandMessage,
|
||||
shouldComputeCommandAuthorized,
|
||||
} from "../../../src/auto-reply/command-detection.js";
|
||||
import { finalizeInboundContext } from "../../../src/auto-reply/reply/inbound-context.js";
|
||||
import { resolveCommandAuthorizedFromAuthorizers } from "../../../src/channels/command-gating.js";
|
||||
@@ -443,16 +443,15 @@ async function processMessageWithPipeline(params: {
|
||||
const dmPolicy = account.config.dmPolicy ?? "pairing";
|
||||
const configAllowFrom = (account.config.allowFrom ?? []).map((v) => String(v));
|
||||
const rawBody = text?.trim() || (mediaPath ? "<media:image>" : "");
|
||||
const shouldComputeCommandAuthorized =
|
||||
isControlCommandMessage(rawBody, config) || hasInlineCommandTokens(rawBody);
|
||||
const shouldComputeAuth = shouldComputeCommandAuthorized(rawBody, config);
|
||||
const storeAllowFrom =
|
||||
!isGroup && (dmPolicy !== "open" || shouldComputeCommandAuthorized)
|
||||
!isGroup && (dmPolicy !== "open" || shouldComputeAuth)
|
||||
? await deps.readChannelAllowFromStore("zalo").catch(() => [])
|
||||
: [];
|
||||
const effectiveAllowFrom = [...configAllowFrom, ...storeAllowFrom];
|
||||
const useAccessGroups = config.commands?.useAccessGroups !== false;
|
||||
const senderAllowedForCommands = isSenderAllowed(senderId, effectiveAllowFrom);
|
||||
const commandAuthorized = shouldComputeCommandAuthorized
|
||||
const commandAuthorized = shouldComputeAuth
|
||||
? resolveCommandAuthorizedFromAuthorizers({
|
||||
useAccessGroups,
|
||||
authorizers: [{ configured: effectiveAllowFrom.length > 0, allowed: senderAllowedForCommands }],
|
||||
|
||||
@@ -2,8 +2,8 @@ import type { ChildProcess } from "node:child_process";
|
||||
|
||||
import type { RuntimeEnv } from "../../../src/runtime.js";
|
||||
import {
|
||||
hasInlineCommandTokens,
|
||||
isControlCommandMessage,
|
||||
shouldComputeCommandAuthorized,
|
||||
} from "../../../src/auto-reply/command-detection.js";
|
||||
import { finalizeInboundContext } from "../../../src/auto-reply/reply/inbound-context.js";
|
||||
import { resolveCommandAuthorizedFromAuthorizers } from "../../../src/channels/command-gating.js";
|
||||
@@ -111,16 +111,15 @@ async function processMessage(
|
||||
const dmPolicy = account.config.dmPolicy ?? "pairing";
|
||||
const configAllowFrom = (account.config.allowFrom ?? []).map((v) => String(v));
|
||||
const rawBody = content.trim();
|
||||
const shouldComputeCommandAuthorized =
|
||||
isControlCommandMessage(rawBody, config) || hasInlineCommandTokens(rawBody);
|
||||
const shouldComputeAuth = shouldComputeCommandAuthorized(rawBody, config);
|
||||
const storeAllowFrom =
|
||||
!isGroup && (dmPolicy !== "open" || shouldComputeCommandAuthorized)
|
||||
!isGroup && (dmPolicy !== "open" || shouldComputeAuth)
|
||||
? await deps.readChannelAllowFromStore("zalouser").catch(() => [])
|
||||
: [];
|
||||
const effectiveAllowFrom = [...configAllowFrom, ...storeAllowFrom];
|
||||
const useAccessGroups = config.commands?.useAccessGroups !== false;
|
||||
const senderAllowedForCommands = isSenderAllowed(senderId, effectiveAllowFrom);
|
||||
const commandAuthorized = shouldComputeCommandAuthorized
|
||||
const commandAuthorized = shouldComputeAuth
|
||||
? resolveCommandAuthorizedFromAuthorizers({
|
||||
useAccessGroups,
|
||||
authorizers: [{ configured: effectiveAllowFrom.length > 0, allowed: senderAllowedForCommands }],
|
||||
|
||||
@@ -58,3 +58,11 @@ export function hasInlineCommandTokens(text?: string): boolean {
|
||||
if (!body.trim()) return false;
|
||||
return /(?:^|\s)[/!][a-z]/i.test(body);
|
||||
}
|
||||
|
||||
export function shouldComputeCommandAuthorized(
|
||||
text?: string,
|
||||
cfg?: ClawdbotConfig,
|
||||
options?: CommandNormalizeOptions,
|
||||
): boolean {
|
||||
return isControlCommandMessage(text, cfg, options) || hasInlineCommandTokens(text);
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import type { ClawdbotConfig } from "../../config/config.js";
|
||||
import { isAbortTrigger, tryFastAbortFromMessage } from "./abort.js";
|
||||
import { enqueueFollowupRun, getFollowupQueueDepth, type FollowupRun } from "./queue.js";
|
||||
import { initSessionState } from "./session.js";
|
||||
import { buildTestCtx } from "./test-ctx.js";
|
||||
|
||||
vi.mock("../../agents/pi-embedded.js", () => ({
|
||||
abortEmbeddedPiRun: vi.fn().mockReturnValue(true),
|
||||
@@ -67,7 +68,7 @@ describe("abort detection", () => {
|
||||
const cfg = { session: { store: storePath }, commands: { text: false } } as ClawdbotConfig;
|
||||
|
||||
const result = await tryFastAbortFromMessage({
|
||||
ctx: {
|
||||
ctx: buildTestCtx({
|
||||
CommandBody: "/stop",
|
||||
RawBody: "/stop",
|
||||
CommandAuthorized: true,
|
||||
@@ -76,7 +77,7 @@ describe("abort detection", () => {
|
||||
Surface: "telegram",
|
||||
From: "telegram:123",
|
||||
To: "telegram:123",
|
||||
},
|
||||
}),
|
||||
cfg,
|
||||
});
|
||||
|
||||
@@ -130,7 +131,7 @@ describe("abort detection", () => {
|
||||
expect(getFollowupQueueDepth(sessionKey)).toBe(1);
|
||||
|
||||
const result = await tryFastAbortFromMessage({
|
||||
ctx: {
|
||||
ctx: buildTestCtx({
|
||||
CommandBody: "/stop",
|
||||
RawBody: "/stop",
|
||||
CommandAuthorized: true,
|
||||
@@ -139,7 +140,7 @@ describe("abort detection", () => {
|
||||
Surface: "telegram",
|
||||
From: "telegram:123",
|
||||
To: "telegram:123",
|
||||
},
|
||||
}),
|
||||
cfg,
|
||||
});
|
||||
|
||||
@@ -187,7 +188,7 @@ describe("abort detection", () => {
|
||||
]);
|
||||
|
||||
const result = await tryFastAbortFromMessage({
|
||||
ctx: {
|
||||
ctx: buildTestCtx({
|
||||
CommandBody: "/stop",
|
||||
RawBody: "/stop",
|
||||
CommandAuthorized: true,
|
||||
@@ -196,7 +197,7 @@ describe("abort detection", () => {
|
||||
Surface: "telegram",
|
||||
From: "telegram:parent",
|
||||
To: "telegram:parent",
|
||||
},
|
||||
}),
|
||||
cfg,
|
||||
});
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
import { parseAgentSessionKey } from "../../routing/session-key.js";
|
||||
import { resolveCommandAuthorization } from "../command-auth.js";
|
||||
import { normalizeCommandBody } from "../commands-registry.js";
|
||||
import type { MsgContext } from "../templating.js";
|
||||
import type { FinalizedMsgContext, MsgContext } from "../templating.js";
|
||||
import { logVerbose } from "../../globals.js";
|
||||
import { stripMentions, stripStructuralPrefixes } from "./mentions.js";
|
||||
import { clearSessionQueues } from "./queue.js";
|
||||
@@ -115,7 +115,7 @@ export function stopSubagentsForRequester(params: {
|
||||
}
|
||||
|
||||
export async function tryFastAbortFromMessage(params: {
|
||||
ctx: MsgContext;
|
||||
ctx: FinalizedMsgContext;
|
||||
cfg: ClawdbotConfig;
|
||||
}): Promise<{ handled: boolean; aborted: boolean; stoppedSubagents?: number }> {
|
||||
const { ctx, cfg } = params;
|
||||
@@ -132,7 +132,7 @@ export async function tryFastAbortFromMessage(params: {
|
||||
const abortRequested = normalized === "/stop" || isAbortTrigger(stripped);
|
||||
if (!abortRequested) return { handled: false, aborted: false };
|
||||
|
||||
const commandAuthorized = ctx.CommandAuthorized ?? false;
|
||||
const commandAuthorized = ctx.CommandAuthorized;
|
||||
const auth = resolveCommandAuthorization({
|
||||
ctx,
|
||||
cfg,
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { ClawdbotConfig } from "../../config/config.js";
|
||||
import type { MsgContext } from "../templating.js";
|
||||
import type { GetReplyOptions, ReplyPayload } from "../types.js";
|
||||
import type { ReplyDispatcher } from "./reply-dispatcher.js";
|
||||
import { buildTestCtx } from "./test-ctx.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
routeReply: vi.fn(async () => ({ ok: true, messageId: "mock" })),
|
||||
@@ -58,11 +59,12 @@ describe("dispatchReplyFromConfig", () => {
|
||||
mocks.routeReply.mockClear();
|
||||
const cfg = {} as ClawdbotConfig;
|
||||
const dispatcher = createDispatcher();
|
||||
const ctx: MsgContext = {
|
||||
const ctx = buildTestCtx({
|
||||
Provider: "slack",
|
||||
Surface: undefined,
|
||||
OriginatingChannel: "slack",
|
||||
OriginatingTo: "channel:C123",
|
||||
};
|
||||
});
|
||||
|
||||
const replyResolver = async (
|
||||
_ctx: MsgContext,
|
||||
@@ -83,13 +85,13 @@ describe("dispatchReplyFromConfig", () => {
|
||||
mocks.routeReply.mockClear();
|
||||
const cfg = {} as ClawdbotConfig;
|
||||
const dispatcher = createDispatcher();
|
||||
const ctx: MsgContext = {
|
||||
const ctx = buildTestCtx({
|
||||
Provider: "slack",
|
||||
AccountId: "acc-1",
|
||||
MessageThreadId: 123,
|
||||
OriginatingChannel: "telegram",
|
||||
OriginatingTo: "telegram:999",
|
||||
};
|
||||
});
|
||||
|
||||
const replyResolver = async (
|
||||
_ctx: MsgContext,
|
||||
@@ -116,10 +118,10 @@ describe("dispatchReplyFromConfig", () => {
|
||||
});
|
||||
const cfg = {} as ClawdbotConfig;
|
||||
const dispatcher = createDispatcher();
|
||||
const ctx: MsgContext = {
|
||||
const ctx = buildTestCtx({
|
||||
Provider: "telegram",
|
||||
Body: "/stop",
|
||||
};
|
||||
});
|
||||
const replyResolver = vi.fn(async () => ({ text: "hi" }) as ReplyPayload);
|
||||
|
||||
await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
|
||||
@@ -138,10 +140,10 @@ describe("dispatchReplyFromConfig", () => {
|
||||
});
|
||||
const cfg = {} as ClawdbotConfig;
|
||||
const dispatcher = createDispatcher();
|
||||
const ctx: MsgContext = {
|
||||
const ctx = buildTestCtx({
|
||||
Provider: "telegram",
|
||||
Body: "/stop",
|
||||
};
|
||||
});
|
||||
|
||||
await dispatchReplyFromConfig({
|
||||
ctx,
|
||||
@@ -161,12 +163,12 @@ describe("dispatchReplyFromConfig", () => {
|
||||
aborted: false,
|
||||
});
|
||||
const cfg = {} as ClawdbotConfig;
|
||||
const ctx: MsgContext = {
|
||||
const ctx = buildTestCtx({
|
||||
Provider: "whatsapp",
|
||||
OriginatingChannel: "whatsapp",
|
||||
OriginatingTo: "whatsapp:+15555550123",
|
||||
MessageSid: "msg-1",
|
||||
};
|
||||
});
|
||||
const replyResolver = vi.fn(async () => ({ text: "hi" }) as ReplyPayload);
|
||||
|
||||
await dispatchReplyFromConfig({
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { ClawdbotConfig } from "../../config/config.js";
|
||||
import { logVerbose } from "../../globals.js";
|
||||
import { getReplyFromConfig } from "../reply.js";
|
||||
import type { MsgContext } from "../templating.js";
|
||||
import type { FinalizedMsgContext } from "../templating.js";
|
||||
import type { GetReplyOptions, ReplyPayload } from "../types.js";
|
||||
import { formatAbortReplyText, tryFastAbortFromMessage } from "./abort.js";
|
||||
import { shouldSkipDuplicateInbound } from "./inbound-dedupe.js";
|
||||
@@ -14,7 +14,7 @@ export type DispatchFromConfigResult = {
|
||||
};
|
||||
|
||||
export async function dispatchReplyFromConfig(params: {
|
||||
ctx: MsgContext;
|
||||
ctx: FinalizedMsgContext;
|
||||
cfg: ClawdbotConfig;
|
||||
dispatcher: ReplyDispatcher;
|
||||
replyOptions?: Omit<GetReplyOptions, "onToolResult" | "onBlockReply">;
|
||||
|
||||
@@ -73,25 +73,25 @@ export async function getReplyFromConfig(
|
||||
silentToken: SILENT_REPLY_TOKEN,
|
||||
log: defaultRuntime.log,
|
||||
});
|
||||
opts?.onTypingController?.(typing);
|
||||
opts?.onTypingController?.(typing);
|
||||
|
||||
finalizeInboundContext(ctx);
|
||||
const finalized = finalizeInboundContext(ctx);
|
||||
|
||||
await applyMediaUnderstanding({
|
||||
ctx,
|
||||
ctx: finalized,
|
||||
cfg,
|
||||
agentDir,
|
||||
activeModel: { provider, model },
|
||||
});
|
||||
|
||||
const commandAuthorized = ctx.CommandAuthorized ?? false;
|
||||
const commandAuthorized = finalized.CommandAuthorized;
|
||||
resolveCommandAuthorization({
|
||||
ctx,
|
||||
ctx: finalized,
|
||||
cfg,
|
||||
commandAuthorized,
|
||||
});
|
||||
const sessionState = await initSessionState({
|
||||
ctx,
|
||||
ctx: finalized,
|
||||
cfg,
|
||||
commandAuthorized,
|
||||
});
|
||||
@@ -113,7 +113,7 @@ export async function getReplyFromConfig(
|
||||
} = sessionState;
|
||||
|
||||
const directiveResult = await resolveReplyDirectives({
|
||||
ctx,
|
||||
ctx: finalized,
|
||||
cfg,
|
||||
agentId,
|
||||
agentDir,
|
||||
|
||||
@@ -18,6 +18,7 @@ describe("finalizeInboundContext", () => {
|
||||
expect(out.RawBody).toBe("raw\nline");
|
||||
expect(out.BodyForAgent).toBe("a\nb\nc");
|
||||
expect(out.BodyForCommands).toBe("raw\nline");
|
||||
expect(out.CommandAuthorized).toBe(false);
|
||||
expect(out.ChatType).toBe("channel");
|
||||
expect(out.ConversationLabel).toContain("Test");
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { normalizeChatType } from "../../channels/chat-type.js";
|
||||
import { resolveConversationLabel } from "../../channels/conversation-label.js";
|
||||
import type { MsgContext } from "../templating.js";
|
||||
import type { FinalizedMsgContext, MsgContext } from "../templating.js";
|
||||
import { formatInboundBodyWithSenderMeta } from "./inbound-sender-meta.js";
|
||||
import { normalizeInboundTextNewlines } from "./inbound-text.js";
|
||||
|
||||
@@ -19,7 +19,7 @@ function normalizeTextField(value: unknown): string | undefined {
|
||||
export function finalizeInboundContext<T extends Record<string, unknown>>(
|
||||
ctx: T,
|
||||
opts: FinalizeInboundContextOptions = {},
|
||||
): T & MsgContext {
|
||||
): T & FinalizedMsgContext {
|
||||
const normalized = ctx as T & MsgContext;
|
||||
|
||||
normalized.Body = normalizeInboundTextNewlines(
|
||||
@@ -64,5 +64,8 @@ export function finalizeInboundContext<T extends Record<string, unknown>>(
|
||||
body: normalized.BodyForAgent,
|
||||
});
|
||||
|
||||
return normalized;
|
||||
// Always set. Default-deny when upstream forgets to populate it.
|
||||
normalized.CommandAuthorized = normalized.CommandAuthorized === true;
|
||||
|
||||
return normalized as T & FinalizedMsgContext;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { ClawdbotConfig } from "../../config/config.js";
|
||||
import type { MsgContext } from "../templating.js";
|
||||
import type { FinalizedMsgContext } from "../templating.js";
|
||||
import type { GetReplyOptions } from "../types.js";
|
||||
import type { DispatchFromConfigResult } from "./dispatch-from-config.js";
|
||||
import { dispatchReplyFromConfig } from "./dispatch-from-config.js";
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
} from "./reply-dispatcher.js";
|
||||
|
||||
export async function dispatchReplyWithBufferedBlockDispatcher(params: {
|
||||
ctx: MsgContext;
|
||||
ctx: FinalizedMsgContext;
|
||||
cfg: ClawdbotConfig;
|
||||
dispatcherOptions: ReplyDispatcherWithTypingOptions;
|
||||
replyOptions?: Omit<GetReplyOptions, "onToolResult" | "onBlockReply">;
|
||||
@@ -37,7 +37,7 @@ export async function dispatchReplyWithBufferedBlockDispatcher(params: {
|
||||
}
|
||||
|
||||
export async function dispatchReplyWithDispatcher(params: {
|
||||
ctx: MsgContext;
|
||||
ctx: FinalizedMsgContext;
|
||||
cfg: ClawdbotConfig;
|
||||
dispatcherOptions: ReplyDispatcherOptions;
|
||||
replyOptions?: Omit<GetReplyOptions, "onToolResult" | "onBlockReply">;
|
||||
|
||||
18
src/auto-reply/reply/test-ctx.ts
Normal file
18
src/auto-reply/reply/test-ctx.ts
Normal file
@@ -0,0 +1,18 @@
|
||||
import type { FinalizedMsgContext, MsgContext } from "../templating.js";
|
||||
import { finalizeInboundContext } from "./inbound-context.js";
|
||||
|
||||
export function buildTestCtx(overrides: Partial<MsgContext> = {}): FinalizedMsgContext {
|
||||
return finalizeInboundContext({
|
||||
Body: "",
|
||||
CommandBody: "",
|
||||
CommandSource: "text",
|
||||
From: "whatsapp:+1000",
|
||||
To: "whatsapp:+2000",
|
||||
ChatType: "direct",
|
||||
Provider: "whatsapp",
|
||||
Surface: "whatsapp",
|
||||
CommandAuthorized: false,
|
||||
...overrides,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -103,6 +103,14 @@ export type MsgContext = {
|
||||
HookMessages?: string[];
|
||||
};
|
||||
|
||||
export type FinalizedMsgContext = Omit<MsgContext, "CommandAuthorized"> & {
|
||||
/**
|
||||
* Always set by finalizeInboundContext().
|
||||
* Default-deny: missing/undefined becomes false.
|
||||
*/
|
||||
CommandAuthorized: boolean;
|
||||
};
|
||||
|
||||
export type TemplateContext = MsgContext & {
|
||||
BodyStripped?: string;
|
||||
SessionId?: string;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { resolveAckReaction } from "../../../agents/identity.js";
|
||||
import { hasControlCommand } from "../../../auto-reply/command-detection.js";
|
||||
import { shouldHandleTextCommands } from "../../../auto-reply/commands-registry.js";
|
||||
import type { FinalizedMsgContext } from "../../../auto-reply/templating.js";
|
||||
import {
|
||||
formatInboundEnvelope,
|
||||
formatThreadStarterEnvelope,
|
||||
@@ -466,10 +467,10 @@ export async function prepareSlackMessage(params: {
|
||||
MediaPath: media?.path,
|
||||
MediaType: media?.contentType,
|
||||
MediaUrl: media?.path,
|
||||
CommandAuthorized: commandAuthorized,
|
||||
OriginatingChannel: "slack" as const,
|
||||
OriginatingTo: slackTo,
|
||||
}) satisfies Record<string, unknown>;
|
||||
CommandAuthorized: commandAuthorized,
|
||||
OriginatingChannel: "slack" as const,
|
||||
OriginatingTo: slackTo,
|
||||
}) satisfies FinalizedMsgContext;
|
||||
|
||||
const replyTarget = ctxPayload.To ?? undefined;
|
||||
if (!replyTarget) return null;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { ResolvedAgentRoute } from "../../../routing/resolve-route.js";
|
||||
import type { FinalizedMsgContext } from "../../../auto-reply/templating.js";
|
||||
import type { ResolvedSlackAccount } from "../../accounts.js";
|
||||
import type { SlackMessageEvent } from "../../types.js";
|
||||
import type { SlackChannelConfigResolved } from "../channel-config.js";
|
||||
@@ -11,7 +12,7 @@ export type PreparedSlackMessage = {
|
||||
route: ResolvedAgentRoute;
|
||||
channelConfig: SlackChannelConfigResolved | null;
|
||||
replyTarget: string;
|
||||
ctxPayload: Record<string, unknown>;
|
||||
ctxPayload: FinalizedMsgContext;
|
||||
isDirectMessage: boolean;
|
||||
isRoomish: boolean;
|
||||
historyKey: string;
|
||||
|
||||
@@ -16,10 +16,7 @@ import {
|
||||
import { dispatchReplyWithBufferedBlockDispatcher } from "../../../auto-reply/reply/provider-dispatcher.js";
|
||||
import type { getReplyFromConfig } from "../../../auto-reply/reply.js";
|
||||
import type { ReplyPayload } from "../../../auto-reply/types.js";
|
||||
import {
|
||||
hasInlineCommandTokens,
|
||||
isControlCommandMessage,
|
||||
} from "../../../auto-reply/command-detection.js";
|
||||
import { shouldComputeCommandAuthorized } from "../../../auto-reply/command-detection.js";
|
||||
import { finalizeInboundContext } from "../../../auto-reply/reply/inbound-context.js";
|
||||
import { toLocationContext } from "../../../channels/location.js";
|
||||
import type { loadConfig } from "../../../config/config.js";
|
||||
@@ -232,9 +229,7 @@ export async function processMessage(params: {
|
||||
const textLimit = params.maxMediaTextChunkLimit ?? resolveTextChunkLimit(params.cfg, "whatsapp");
|
||||
let didLogHeartbeatStrip = false;
|
||||
let didSendReply = false;
|
||||
const shouldComputeCommandAuthorized =
|
||||
isControlCommandMessage(params.msg.body, params.cfg) || hasInlineCommandTokens(params.msg.body);
|
||||
const commandAuthorized = shouldComputeCommandAuthorized
|
||||
const commandAuthorized = shouldComputeCommandAuthorized(params.msg.body, params.cfg)
|
||||
? await resolveWhatsAppCommandAuthorized({ cfg: params.cfg, msg: params.msg })
|
||||
: undefined;
|
||||
const configuredResponsePrefix = params.cfg.messages?.responsePrefix;
|
||||
|
||||
Reference in New Issue
Block a user