negodiy/openclaw
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

security(feishu): bind doc create grants to trusted requester context (#31184)

Browse Source 
Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
Tak Hoffman
2026-03-01 20:51:45 -06:00
committed by GitHub
parent e482da6682
commit bbab94c1fe
9 changed files with 142 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -127,6 +127,7 @@ Docs: https://docs.openclaw.ai
- Telegram/Group allowlist ordering: evaluate chat allowlist before sender allowlist enforcement so explicitly allowlisted groups are not fail-closed by empty sender allowlists. Landed from contributor PR #30680 by @openperf. Thanks @openperf.
- Telegram/Multi-account group isolation: prevent channel-level `groups` config from leaking across Telegram accounts in multi-account setups, avoiding cross-account group routing drops. Landed from contributor PR #30677 by @YUJIE2002. Thanks @YUJIE2002.
- Telegram/Voice caption overflow fallback: recover from `sendVoice` caption length errors by re-sending voice without caption and delivering text separately so replies are not lost. Landed from contributor PR #31131 by @Sid-Qin. Thanks @Sid-Qin.
- Feishu/Doc create permissions: remove caller-controlled owner fields from `feishu_doc` create and bind optional grant behavior to trusted Feishu requester context (`grant_to_requester`), preventing principal selection via tool arguments. (#31184) Thanks @Takhoffman.
- Routing/Binding peer-kind parity: treat `peer.kind` `group` and `channel` as equivalent for binding scope matching (while keeping `direct` separate) so Slack/public channel bindings do not silently fall through. Landed from contributor PR #31135 by @Sid-Qin. Thanks @Sid-Qin.
- Cron/Store EBUSY fallback: retry `rename` on `EBUSY` and use `copyFile` fallback on Windows when replacing cron store files so busy-file contention no longer causes false write failures. (#16932) Thanks @sudhanva-chakra.
- Agents/FS workspace default: honor documented host file-tool default `tools.fs.workspaceOnly=false` when unset so host `write`/`edit` calls are not incorrectly workspace-restricted unless explicitly enabled. Landed from contributor PR #31128 by @SaucePackets. Thanks @SaucePackets.

10
extensions/feishu/src/doc-schema.ts
View File

@@ -29,12 +29,10 @@ export const FeishuDocSchema = Type.Union([
action: Type.Literal("create"),
title: Type.String({ description: "Document title" }),
folder_token: Type.Optional(Type.String({ description: "Target folder token (optional)" })),
owner_open_id: Type.Optional(
Type.String({ description: "Open ID of the user to grant ownership permission" }),
),
owner_perm_type: Type.Optional(
Type.Union([Type.Literal("view"), Type.Literal("edit"), Type.Literal("full_access")], {
description: "Permission type (default: full_access)",
grant_to_requester: Type.Optional(
Type.Boolean({
description:
"Grant edit permission to the trusted requesting Feishu user from runtime context (default: true).",
}),
),
}),

108
extensions/feishu/src/docx.test.ts
View File

@@ -340,7 +340,7 @@ describe("feishu_doc image fetch hardening", () => {
consoleErrorSpy.mockRestore();
});
it("reports owner permission details when grant succeeds", async () => {
it("create grants permission only to trusted Feishu requester", async () => {
const registerTool = vi.fn();
registerFeishuDocTools({
config: {
@@ -357,27 +357,35 @@ describe("feishu_doc image fetch hardening", () => {
const feishuDocTool = registerTool.mock.calls
.map((call) => call[0])
.map((tool) => (typeof tool === "function" ? tool({}) : tool))
.map((tool) =>
typeof tool === "function"
? tool({ messageChannel: "feishu", requesterSenderId: "ou_123" })
: tool,
)
.find((tool) => tool.name === "feishu_doc");
expect(feishuDocTool).toBeDefined();
const result = await feishuDocTool.execute("tool-call", {
action: "create",
title: "Demo",
owner_open_id: "ou_123",
owner_perm_type: "edit",
});
expect(permissionMemberCreateMock).toHaveBeenCalled();
expect(result.details.owner_permission_added).toBe(true);
expect(result.details.owner_open_id).toBe("ou_123");
expect(result.details.owner_perm_type).toBe("edit");
expect(result.details.document_id).toBe("doc_created");
expect(result.details.requester_permission_added).toBe(true);
expect(result.details.requester_open_id).toBe("ou_123");
expect(result.details.requester_perm_type).toBe("edit");
expect(permissionMemberCreateMock).toHaveBeenCalledWith(
expect.objectContaining({
data: expect.objectContaining({
member_type: "openid",
member_id: "ou_123",
perm: "edit",
}),
}),
);
});
it("does not report owner permission details when grant fails", async () => {
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
permissionMemberCreateMock.mockRejectedValueOnce(new Error("permission denied"));
it("create skips requester grant when trusted requester identity is unavailable", async () => {
const registerTool = vi.fn();
registerFeishuDocTools({
config: {
@@ -394,43 +402,7 @@ describe("feishu_doc image fetch hardening", () => {
const feishuDocTool = registerTool.mock.calls
.map((call) => call[0])
.map((tool) => (typeof tool === "function" ? tool({}) : tool))
.find((tool) => tool.name === "feishu_doc");
expect(feishuDocTool).toBeDefined();
const result = await feishuDocTool.execute("tool-call", {
action: "create",
title: "Demo",
owner_open_id: "ou_123",
owner_perm_type: "edit",
});
expect(permissionMemberCreateMock).toHaveBeenCalled();
expect(result.details.owner_permission_added).toBeUndefined();
expect(result.details.owner_open_id).toBeUndefined();
expect(result.details.owner_perm_type).toBeUndefined();
expect(consoleWarnSpy).toHaveBeenCalled();
consoleWarnSpy.mockRestore();
});
it("skips permission grant when owner_open_id is omitted", async () => {
const registerTool = vi.fn();
registerFeishuDocTools({
config: {
channels: {
feishu: {
appId: "app_id",
appSecret: "app_secret",
},
},
} as any,
logger: { debug: vi.fn(), info: vi.fn() } as any,
registerTool,
} as any);
const feishuDocTool = registerTool.mock.calls
.map((call) => call[0])
.map((tool) => (typeof tool === "function" ? tool({}) : tool))
.map((tool) => (typeof tool === "function" ? tool({ messageChannel: "feishu" }) : tool))
.find((tool) => tool.name === "feishu_doc");
expect(feishuDocTool).toBeDefined();
@@ -440,7 +412,43 @@ describe("feishu_doc image fetch hardening", () => {
});
expect(permissionMemberCreateMock).not.toHaveBeenCalled();
expect(result.details.owner_permission_added).toBeUndefined();
expect(result.details.requester_permission_added).toBe(false);
expect(result.details.requester_permission_skipped_reason).toContain("trusted requester");
});
it("create never grants permissions when grant_to_requester is false", async () => {
const registerTool = vi.fn();
registerFeishuDocTools({
config: {
channels: {
feishu: {
appId: "app_id",
appSecret: "app_secret",
},
},
} as any,
logger: { debug: vi.fn(), info: vi.fn() } as any,
registerTool,
} as any);
const feishuDocTool = registerTool.mock.calls
.map((call) => call[0])
.map((tool) =>
typeof tool === "function"
? tool({ messageChannel: "feishu", requesterSenderId: "ou_123" })
: tool,
)
.find((tool) => tool.name === "feishu_doc");
expect(feishuDocTool).toBeDefined();
const result = await feishuDocTool.execute("tool-call", {
action: "create",
title: "Demo",
grant_to_requester: false,
});
expect(permissionMemberCreateMock).not.toHaveBeenCalled();
expect(result.details.requester_permission_added).toBeUndefined();
});
it("returns an error when create response omits document_id", async () => {

70
extensions/feishu/src/docx.ts
View File

@@ -751,8 +751,7 @@ async function createDoc(
client: Lark.Client,
title: string,
folderToken?: string,
ownerOpenId?: string,
ownerPermType: "view" | "edit" | "full_access" = "full_access",
options?: { grantToRequester?: boolean; requesterOpenId?: string },
) {
const res = await client.docx.document.create({
data: { title, folder_token: folderToken },
@@ -765,23 +764,32 @@ async function createDoc(
if (!docToken) {
throw new Error("Document creation succeeded but no document_id was returned");
}
let ownerPermissionAdded = false;
const shouldGrantToRequester = options?.grantToRequester !== false;
const requesterOpenId = options?.requesterOpenId?.trim();
const requesterPermType: "edit" = "edit";
// Auto add owner permission if ownerOpenId is provided
if (docToken && ownerOpenId) {
try {
await client.drive.permissionMember.create({
path: { token: docToken },
params: { type: "docx", need_notification: false },
data: {
member_type: "openid",
member_id: ownerOpenId,
perm: ownerPermType,
},
});
ownerPermissionAdded = true;
} catch (err) {
console.warn("Failed to add owner permission (non-critical):", err);
let requesterPermissionAdded = false;
let requesterPermissionSkippedReason: string | undefined;
let requesterPermissionError: string | undefined;
if (shouldGrantToRequester) {
if (!requesterOpenId) {
requesterPermissionSkippedReason = "trusted requester identity unavailable";
} else {
try {
await client.drive.permissionMember.create({
path: { token: docToken },
params: { type: "docx", need_notification: false },
data: {
member_type: "openid",
member_id: requesterOpenId,
perm: requesterPermType,
},
});
requesterPermissionAdded = true;
} catch (err) {
requesterPermissionError = err instanceof Error ? err.message : String(err);
}
}
}
@@ -789,12 +797,15 @@ async function createDoc(
document_id: docToken,
title: doc?.title,
url: `https://feishu.cn/docx/${docToken}`,
...(ownerOpenId &&
ownerPermissionAdded && {
owner_permission_added: true,
owner_open_id: ownerOpenId,
owner_perm_type: ownerPermType,
...(shouldGrantToRequester && {
requester_permission_added: requesterPermissionAdded,
...(requesterOpenId && { requester_open_id: requesterOpenId }),
requester_perm_type: requesterPermType,
...(requesterPermissionSkippedReason && {
requester_permission_skipped_reason: requesterPermissionSkippedReason,
}),
...(requesterPermissionError && { requester_permission_error: requesterPermissionError }),
}),
};
}
@@ -1251,6 +1262,8 @@ export function registerFeishuDocTools(api: OpenClawPluginApi) {
api.registerTool(
(ctx) => {
const defaultAccountId = ctx.agentAccountId;
const trustedRequesterOpenId =
ctx.messageChannel === "feishu" ? ctx.requesterSenderId?.trim() || undefined : undefined;
return {
name: "feishu_doc",
label: "Feishu Doc",
@@ -1297,13 +1310,10 @@ export function registerFeishuDocTools(api: OpenClawPluginApi) {
);
case "create":
return json(
await createDoc(
client,
p.title,
p.folder_token,
p.owner_open_id,
p.owner_perm_type,
),
await createDoc(client, p.title, p.folder_token, {
grantToRequester: p.grant_to_requester,
requesterOpenId: trustedRequesterOpenId,
}),
);
case "list_blocks":
return json(await listBlocks(client, p.doc_token));

28
src/agents/openclaw-tools.plugin-context.test.ts Normal file
View File

@@ -0,0 +1,28 @@
import { describe, expect, it, vi } from "vitest";
const resolvePluginToolsMock = vi.fn(() => []);
vi.mock("../plugins/tools.js", () => ({
resolvePluginTools: (params: unknown) => resolvePluginToolsMock(params),
}));
import { createOpenClawTools } from "./openclaw-tools.js";
describe("createOpenClawTools plugin context", () => {
it("forwards trusted requester sender identity to plugin tool context", () => {
createOpenClawTools({
config: {} as never,
requesterSenderId: "trusted-sender",
senderIsOwner: true,
});
expect(resolvePluginToolsMock).toHaveBeenCalledWith(
expect.objectContaining({
context: expect.objectContaining({
requesterSenderId: "trusted-sender",
senderIsOwner: true,
}),
}),
);
});
});

2
src/agents/openclaw-tools.ts
View File

@@ -187,6 +187,8 @@ export function createOpenClawTools(options?: {
sessionKey: options?.agentSessionKey,
messageChannel: options?.agentChannel,
agentAccountId: options?.agentAccountId,
requesterSenderId: options?.requesterSenderId ?? undefined,
senderIsOwner: options?.senderIsOwner ?? undefined,
sandboxed: options?.sandboxed,
},
existingToolNames: new Set(tools.map((tool) => tool.name)),

4
src/plugins/types.ts
View File

@@ -63,6 +63,10 @@ export type OpenClawPluginToolContext = {
sessionKey?: string;
messageChannel?: string;
agentAccountId?: string;
/** Trusted sender id from inbound context (runtime-provided, not tool args). */
requesterSenderId?: string;
/** Whether the trusted sender is an owner. */
senderIsOwner?: boolean;
sandboxed?: boolean;
};

4
src/security/audit.test.ts
View File

@@ -1266,7 +1266,7 @@ describe("security audit", () => {
);
});
it("warns when Feishu doc tool is enabled because create supports owner_open_id", async () => {
it("warns when Feishu doc tool is enabled because create can grant requester access", async () => {
const cfg: OpenClawConfig = {
channels: {
feishu: {
@@ -1280,7 +1280,7 @@ describe("security audit", () => {
expectFinding(res, "channels.feishu.doc_owner_open_id", "warn");
});
it("does not warn for Feishu owner_open_id when doc tools are disabled", async () => {
it("does not warn for Feishu doc grant risk when doc tools are disabled", async () => {
const cfg: OpenClawConfig = {
channels: {
feishu: {

6
src/security/audit.ts
View File

@@ -519,11 +519,11 @@ function collectGatewayConfigFindings(
findings.push({
checkId: "channels.feishu.doc_owner_open_id",
severity: "warn",
title: "Feishu doc create can grant owner permissions",
title: "Feishu doc create can grant requester permissions",
detail:
'channels.feishu tools include "doc"; feishu_doc action "create" accepts owner_open_id and can grant document access to that user.',
'channels.feishu tools include "doc"; feishu_doc action "create" can grant document access to the trusted requesting Feishu user.',
remediation:
"Disable channels.feishu.tools.doc when not needed, and restrict tool access so untrusted prompts cannot set owner_open_id.",
"Disable channels.feishu.tools.doc when not needed, and restrict tool access for untrusted prompts.",
});
}