mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-19 05:50:47 +00:00
fix(tlon): defer DM cite expansion until after auth
This commit is contained in:
@@ -101,6 +101,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Docs/Mintlify: fix MDX marker syntax on Perplexity, Model Providers, Moonshot, and exec approvals pages so local docs preview no longer breaks rendering or leaves stale pages unpublished. (#46695) Thanks @velvet-shark.
|
||||
- Gateway/config validation: stop treating the implicit default memory slot as a required explicit plugin config, so startup no longer fails with `plugins.slots.memory: plugin not found: memory-core` when `memory-core` was only inferred. (#47494) Thanks @ngutman.
|
||||
- Tlon: honor explicit empty allowlists and defer cite expansion. (#46788) Thanks @zpbrent and @vincentkoc.
|
||||
- Tlon/DM auth: defer cited-message expansion until after DM authorization and owner command handling, so unauthorized DMs and owner approval/admin commands no longer trigger cross-channel cite fetches before the deny or command path.
|
||||
- Nodes/pending actions: re-check queued foreground actions against the current node command policy before returning them to the node. (#46815) Thanks @zpbrent and @vincentkoc.
|
||||
- Node/startup: remove leftover debug `console.log("node host PATH: ...")` that printed the resolved PATH on every `openclaw node run` invocation. (#46515) Fixes #46411. Thanks @ademczuk.
|
||||
- CLI/completion: reduce recursive completion-script string churn and fix nested PowerShell command-path matching so generated nested completions resolve on PowerShell too. (#45537) Thanks @yiShanXin and @vincentkoc.
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
stripBotMention,
|
||||
isDmAllowed,
|
||||
isSummarizationRequest,
|
||||
resolveAuthorizedMessageText,
|
||||
type ParsedCite,
|
||||
} from "./utils.js";
|
||||
|
||||
@@ -1245,9 +1246,12 @@ export async function monitorTlonProvider(opts: MonitorTlonOpts = {}): Promise<v
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve quoted content only after the sender passed channel authorization.
|
||||
const citedContent = await resolveAllCites(content.content);
|
||||
const messageText = citedContent + rawText;
|
||||
const messageText = await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content: content.content,
|
||||
authorizedForCites: true,
|
||||
resolveAllCites,
|
||||
});
|
||||
|
||||
const parsed = parseChannelNest(nest);
|
||||
await processMessage({
|
||||
@@ -1370,8 +1374,6 @@ export async function monitorTlonProvider(opts: MonitorTlonOpts = {}): Promise<v
|
||||
if (!rawText.trim()) {
|
||||
return;
|
||||
}
|
||||
const citedContent = await resolveAllCites(essay.content);
|
||||
const resolvedMessageText = citedContent + rawText;
|
||||
|
||||
// Check if this is the owner sending an approval response
|
||||
const messageText = rawText;
|
||||
@@ -1394,6 +1396,12 @@ export async function monitorTlonProvider(opts: MonitorTlonOpts = {}): Promise<v
|
||||
|
||||
// Owner is always allowed to DM (bypass allowlist)
|
||||
if (isOwner(senderShip)) {
|
||||
const resolvedMessageText = await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content: essay.content,
|
||||
authorizedForCites: true,
|
||||
resolveAllCites,
|
||||
});
|
||||
runtime.log?.(`[tlon] Processing DM from owner ${senderShip}`);
|
||||
await processMessage({
|
||||
messageId: messageId ?? "",
|
||||
@@ -1429,9 +1437,14 @@ export async function monitorTlonProvider(opts: MonitorTlonOpts = {}): Promise<v
|
||||
}
|
||||
|
||||
await processMessage({
|
||||
messageText: await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content: essay.content,
|
||||
authorizedForCites: true,
|
||||
resolveAllCites,
|
||||
}),
|
||||
messageId: messageId ?? "",
|
||||
senderShip,
|
||||
messageText: resolvedMessageText,
|
||||
messageContent: essay.content, // Pass raw content for media extraction
|
||||
isGroup: false,
|
||||
timestamp: essay.sent || Date.now(),
|
||||
|
||||
@@ -161,6 +161,24 @@ export function isGroupInviteAllowed(
|
||||
return allowlist.map((ship) => normalizeShip(ship)).some((ship) => ship === normalizedInviter);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve quoted/cited content only after the caller has passed authorization.
|
||||
* Unauthorized paths must keep raw text and must not trigger cross-channel cite fetches.
|
||||
*/
|
||||
export async function resolveAuthorizedMessageText(params: {
|
||||
rawText: string;
|
||||
content: unknown;
|
||||
authorizedForCites: boolean;
|
||||
resolveAllCites: (content: unknown) => Promise<string>;
|
||||
}): Promise<string> {
|
||||
const { rawText, content, authorizedForCites, resolveAllCites } = params;
|
||||
if (!authorizedForCites) {
|
||||
return rawText;
|
||||
}
|
||||
const citedContent = await resolveAllCites(content);
|
||||
return citedContent + rawText;
|
||||
}
|
||||
|
||||
// Helper to recursively extract text from inline content
|
||||
function renderInlineItem(
|
||||
item: any,
|
||||
|
||||
@@ -8,12 +8,14 @@
|
||||
* - Bot mention detection boundaries
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
extractCites,
|
||||
isDmAllowed,
|
||||
isGroupInviteAllowed,
|
||||
isBotMentioned,
|
||||
extractMessageText,
|
||||
resolveAuthorizedMessageText,
|
||||
} from "./monitor/utils.js";
|
||||
import { normalizeShip } from "./targets.js";
|
||||
|
||||
@@ -340,6 +342,186 @@ describe("Security: Authorization Edge Cases", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("Security: Cite Resolution Authorization Ordering", () => {
|
||||
async function resolveAllCitesForPoC(
|
||||
content: unknown,
|
||||
api: { scry: (path: string) => Promise<unknown> },
|
||||
): Promise<string> {
|
||||
const cites = extractCites(content);
|
||||
if (cites.length === 0) {
|
||||
return "";
|
||||
}
|
||||
|
||||
const resolved: string[] = [];
|
||||
for (const cite of cites) {
|
||||
if (cite.type !== "chan" || !cite.nest || !cite.postId) {
|
||||
continue;
|
||||
}
|
||||
const data = (await api.scry(`/channels/v4/${cite.nest}/posts/post/${cite.postId}.json`)) as {
|
||||
essay?: { content?: unknown };
|
||||
};
|
||||
const text = data?.essay?.content ? extractMessageText(data.essay.content) : "";
|
||||
if (text) {
|
||||
resolved.push(`> ${cite.author || "unknown"} wrote: ${text}`);
|
||||
}
|
||||
}
|
||||
|
||||
return resolved.length > 0 ? resolved.join("\n") + "\n\n" : "";
|
||||
}
|
||||
|
||||
function buildCitedMessage(
|
||||
secretNest = "chat/~private-ship/ops",
|
||||
postId = "1701411845077995094",
|
||||
) {
|
||||
return [
|
||||
{
|
||||
block: {
|
||||
cite: {
|
||||
chan: {
|
||||
nest: secretNest,
|
||||
where: `/msg/~victim-ship/${postId}`,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{ inline: ["~bot-ship please summarize this"] },
|
||||
];
|
||||
}
|
||||
|
||||
it("does not resolve channel cites for unauthorized senders", async () => {
|
||||
const content = buildCitedMessage();
|
||||
const rawText = extractMessageText(content);
|
||||
const api = {
|
||||
scry: vi.fn(async () => ({
|
||||
essay: { content: [{ inline: ["TOP-SECRET"] }] },
|
||||
})),
|
||||
};
|
||||
|
||||
const messageText = await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content,
|
||||
authorizedForCites: false,
|
||||
resolveAllCites: (nextContent) => resolveAllCitesForPoC(nextContent, api),
|
||||
});
|
||||
|
||||
expect(messageText).toBe(rawText);
|
||||
expect(api.scry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("resolves channel cites after sender authorization passes", async () => {
|
||||
const secretNest = "chat/~private-ship/ops";
|
||||
const postId = "170141184507799509469114119040828178432";
|
||||
const content = buildCitedMessage(secretNest, postId);
|
||||
const rawText = extractMessageText(content);
|
||||
const api = {
|
||||
scry: vi.fn(async (path: string) => {
|
||||
expect(path).toBe(`/channels/v4/${secretNest}/posts/post/${postId}.json`);
|
||||
return {
|
||||
essay: { content: [{ inline: ["TOP-SECRET: migration key is rotate-me"] }] },
|
||||
};
|
||||
}),
|
||||
};
|
||||
|
||||
const messageText = await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content,
|
||||
authorizedForCites: true,
|
||||
resolveAllCites: (nextContent) => resolveAllCitesForPoC(nextContent, api),
|
||||
});
|
||||
|
||||
expect(api.scry).toHaveBeenCalledTimes(1);
|
||||
expect(messageText).toContain("TOP-SECRET: migration key is rotate-me");
|
||||
expect(messageText).toContain("> ~victim-ship wrote: TOP-SECRET: migration key is rotate-me");
|
||||
});
|
||||
|
||||
it("does not resolve DM cites before a deny path", async () => {
|
||||
const content = buildCitedMessage("chat/~secret-dm/ops", "1701411845077995095");
|
||||
const rawText = extractMessageText(content);
|
||||
const senderShip = "~attacker-ship";
|
||||
const allowlist = ["~trusted-ship"];
|
||||
const api = {
|
||||
scry: vi.fn(async () => ({
|
||||
essay: { content: [{ inline: ["DM-SECRET"] }] },
|
||||
})),
|
||||
};
|
||||
|
||||
const senderAllowed = allowlist
|
||||
.map((ship) => normalizeShip(ship))
|
||||
.includes(normalizeShip(senderShip));
|
||||
expect(senderAllowed).toBe(false);
|
||||
|
||||
const messageText = await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content,
|
||||
authorizedForCites: senderAllowed,
|
||||
resolveAllCites: (nextContent) => resolveAllCitesForPoC(nextContent, api),
|
||||
});
|
||||
|
||||
expect(messageText).toBe(rawText);
|
||||
expect(api.scry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not resolve DM cites before owner approval command handling", async () => {
|
||||
const content = [
|
||||
{
|
||||
block: {
|
||||
cite: {
|
||||
chan: {
|
||||
nest: "chat/~private-ship/admin",
|
||||
where: "/msg/~victim-ship/1701411845077995096",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{ inline: ["/approve 1"] },
|
||||
];
|
||||
const rawText = extractMessageText(content);
|
||||
const api = {
|
||||
scry: vi.fn(async () => ({
|
||||
essay: { content: [{ inline: ["ADMIN-SECRET"] }] },
|
||||
})),
|
||||
};
|
||||
|
||||
const messageText = await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content,
|
||||
authorizedForCites: false,
|
||||
resolveAllCites: (nextContent) => resolveAllCitesForPoC(nextContent, api),
|
||||
});
|
||||
|
||||
expect(rawText).toContain("/approve 1");
|
||||
expect(messageText).toBe(rawText);
|
||||
expect(messageText).not.toContain("ADMIN-SECRET");
|
||||
expect(api.scry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("resolves DM cites for allowed senders after authorization passes", async () => {
|
||||
const secretNest = "chat/~private-ship/dm";
|
||||
const postId = "1701411845077995097";
|
||||
const content = buildCitedMessage(secretNest, postId);
|
||||
const rawText = extractMessageText(content);
|
||||
const api = {
|
||||
scry: vi.fn(async (path: string) => {
|
||||
expect(path).toBe(`/channels/v4/${secretNest}/posts/post/${postId}.json`);
|
||||
return {
|
||||
essay: { content: [{ inline: ["ALLOWED-DM-SECRET"] }] },
|
||||
};
|
||||
}),
|
||||
};
|
||||
|
||||
const messageText = await resolveAuthorizedMessageText({
|
||||
rawText,
|
||||
content,
|
||||
authorizedForCites: true,
|
||||
resolveAllCites: (nextContent) => resolveAllCitesForPoC(nextContent, api),
|
||||
});
|
||||
|
||||
expect(api.scry).toHaveBeenCalledTimes(1);
|
||||
expect(messageText).toContain("ALLOWED-DM-SECRET");
|
||||
expect(messageText).toContain("> ~victim-ship wrote: ALLOWED-DM-SECRET");
|
||||
});
|
||||
});
|
||||
|
||||
describe("Security: Sender Role Identification", () => {
|
||||
/**
|
||||
* Tests for sender role identification (owner vs user).
|
||||
|
||||
Reference in New Issue
Block a user