From 9692dc7668cb3886562ca264a5853964f917f8db Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 12 Mar 2026 22:27:44 +0000 Subject: [PATCH] fix(security): harden nodes owner-only tool gating --- CHANGELOG.md | 1 + SECURITY.md | 3 +++ ...openclaw-tools.owner-authorization.test.ts | 22 +++++++++++++++++++ .../pi-tools.whatsapp-login-gating.test.ts | 10 +++++++++ src/agents/tools/nodes-tool.test.ts | 5 +++++ src/agents/tools/nodes-tool.ts | 1 + 6 files changed, 42 insertions(+) create mode 100644 src/agents/openclaw-tools.owner-authorization.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d001a9b03..fc0ea1136 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai - Security/browser.request: block persistent browser profile create/delete routes from write-scoped `browser.request` so callers can no longer persist admin-only browser profile changes through the browser control surface. (`GHSA-vmhq-cqm9-6p7q`)(#43800) Thanks @tdjackey and @vincentkoc. - Security/agent: reject public spawned-run lineage fields and keep workspace inheritance on the internal spawned-session path so external `agent` callers can no longer override the gateway workspace boundary. (`GHSA-2rqg-gjgv-84jm`)(#43801) Thanks @tdjackey and @vincentkoc. - Security/session_status: enforce sandbox session-tree visibility and shared agent-to-agent access guards before reading or mutating target session state, so sandboxed subagents can no longer inspect parent session metadata or write parent model overrides via `session_status`. (`GHSA-wcxr-59v9-rxr8`)(#43754) Thanks @tdjackey and @vincentkoc. +- Security/agent tools: mark `nodes` as explicitly owner-only and document/test that `canvas` remains a shared trusted-operator surface unless a real boundary bypass exists. - Security/device pairing: cap issued and verified device-token scopes to each paired device's approved scope baseline so stale or overbroad tokens cannot exceed approved access. (`GHSA-2pwv-x786-56f8`)(#43686) Thanks @tdjackey and @vincentkoc. - Models/secrets: enforce source-managed SecretRef markers in generated `models.json` so runtime-resolved provider secrets are not persisted when runtime projection is skipped. (#43759) Thanks @joshavant. - Security/WebSocket preauth: shorten unauthenticated handshake retention and reject oversized pre-auth frames before application-layer parsing to reduce pre-pairing exposure on unsupported public deployments. (`GHSA-jv4g-m82p-2j93`)(#44089) (`GHSA-xwx2-ppv2-wx98`)(#44089) Thanks @ez-lbz and @vincentkoc. diff --git a/SECURITY.md b/SECURITY.md index b8c3b4149..bef814525 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -37,6 +37,7 @@ For fastest triage, include all of the following: - Exact vulnerable path (`file`, function, and line range) on a current revision. - Tested version details (OpenClaw version and/or commit SHA). - Reproducible PoC against latest `main` or latest released version. +- If the claim targets a released version, evidence from the shipped tag and published artifact/package for that exact version (not only `main`). - Demonstrated impact tied to OpenClaw's documented trust boundaries. - For exposed-secret reports: proof the credential is OpenClaw-owned (or grants access to OpenClaw-operated infrastructure/services). - Explicit statement that the report does not rely on adversarial operators sharing one gateway host/config. @@ -66,6 +67,7 @@ These are frequently reported but are typically closed with no code change: - Discord inbound webhook signature findings for paths not used by this repo's Discord integration. - Claims that Microsoft Teams `fileConsent/invoke` `uploadInfo.uploadUrl` is attacker-controlled without demonstrating one of: auth boundary bypass, a real authenticated Teams/Bot Framework event carrying attacker-chosen URL, or compromise of the Microsoft/Bot trust path. - Scanner-only claims against stale/nonexistent paths, or claims without a working repro. +- Reports that restate an already-fixed issue against later released versions without showing the vulnerable path still exists in the shipped tag or published artifact for that later version. ### Duplicate Report Handling @@ -147,6 +149,7 @@ OpenClaw security guidance assumes: OpenClaw's security model is "personal assistant" (one trusted operator, potentially many agents), not "shared multi-tenant bus." - If multiple people can message the same tool-enabled agent (for example a shared Slack workspace), they can all steer that agent within its granted permissions. +- Non-owner sender status only affects owner-only tools/commands. If a non-owner can still access a non-owner-only tool on that same agent (for example `canvas`), that is within the granted tool boundary unless the report demonstrates an auth, policy, allowlist, approval, or sandbox bypass. - Session or memory scoping reduces context bleed, but does **not** create per-user host authorization boundaries. - For mixed-trust or adversarial users, isolate by OS user/host/gateway and use separate credentials per boundary. - A company-shared agent can be a valid setup when users are in the same trust boundary and the agent is strictly business-only. diff --git a/src/agents/openclaw-tools.owner-authorization.test.ts b/src/agents/openclaw-tools.owner-authorization.test.ts new file mode 100644 index 000000000..47892235b --- /dev/null +++ b/src/agents/openclaw-tools.owner-authorization.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import "./test-helpers/fast-core-tools.js"; +import { createOpenClawTools } from "./openclaw-tools.js"; + +function readToolByName() { + return new Map(createOpenClawTools().map((tool) => [tool.name, tool])); +} + +describe("createOpenClawTools owner authorization", () => { + it("marks owner-only core tools in raw registration", () => { + const tools = readToolByName(); + expect(tools.get("cron")?.ownerOnly).toBe(true); + expect(tools.get("gateway")?.ownerOnly).toBe(true); + expect(tools.get("nodes")?.ownerOnly).toBe(true); + }); + + it("keeps canvas non-owner-only in raw registration", () => { + const tools = readToolByName(); + expect(tools.get("canvas")).toBeDefined(); + expect(tools.get("canvas")?.ownerOnly).not.toBe(true); + }); +}); diff --git a/src/agents/pi-tools.whatsapp-login-gating.test.ts b/src/agents/pi-tools.whatsapp-login-gating.test.ts index 61f65fc05..8dd6637be 100644 --- a/src/agents/pi-tools.whatsapp-login-gating.test.ts +++ b/src/agents/pi-tools.whatsapp-login-gating.test.ts @@ -21,6 +21,7 @@ describe("owner-only tool gating", () => { expect(toolNames).not.toContain("whatsapp_login"); expect(toolNames).not.toContain("cron"); expect(toolNames).not.toContain("gateway"); + expect(toolNames).not.toContain("nodes"); }); it("keeps owner-only tools for authorized senders", () => { @@ -29,6 +30,13 @@ describe("owner-only tool gating", () => { expect(toolNames).toContain("whatsapp_login"); expect(toolNames).toContain("cron"); expect(toolNames).toContain("gateway"); + expect(toolNames).toContain("nodes"); + }); + + it("keeps canvas available to unauthorized senders by current trust model", () => { + const tools = createOpenClawCodingTools({ senderIsOwner: false }); + const toolNames = tools.map((tool) => tool.name); + expect(toolNames).toContain("canvas"); }); it("defaults to removing owner-only tools when owner status is unknown", () => { @@ -37,5 +45,7 @@ describe("owner-only tool gating", () => { expect(toolNames).not.toContain("whatsapp_login"); expect(toolNames).not.toContain("cron"); expect(toolNames).not.toContain("gateway"); + expect(toolNames).not.toContain("nodes"); + expect(toolNames).toContain("canvas"); }); }); diff --git a/src/agents/tools/nodes-tool.test.ts b/src/agents/tools/nodes-tool.test.ts index ddde0b850..2a98973f6 100644 --- a/src/agents/tools/nodes-tool.test.ts +++ b/src/agents/tools/nodes-tool.test.ts @@ -53,6 +53,11 @@ describe("createNodesTool screen_record duration guardrails", () => { screenMocks.writeScreenRecordToFile.mockClear(); }); + it("marks nodes as owner-only", () => { + const tool = createNodesTool(); + expect(tool.ownerOnly).toBe(true); + }); + it("caps durationMs schema at 300000", () => { const tool = createNodesTool(); const schema = tool.parameters as { diff --git a/src/agents/tools/nodes-tool.ts b/src/agents/tools/nodes-tool.ts index e57ff735c..d6f4832d9 100644 --- a/src/agents/tools/nodes-tool.ts +++ b/src/agents/tools/nodes-tool.ts @@ -175,6 +175,7 @@ export function createNodesTool(options?: { return { label: "Nodes", name: "nodes", + ownerOnly: true, description: "Discover and control paired nodes (status/describe/pairing/notify/camera/photos/screen/location/notifications/run/invoke).", parameters: NodesToolSchema,