From 99a02bf115cfc4a594885bc170c877ae182ff352 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 12:13:49 -0700 Subject: [PATCH] feat(approvals): typed approval scope summaries on channel cards (#130116) * feat(approvals): typed approval scope summaries on channel cards Approval owners can attach a closed ApprovalScope union (message-send, payment, external-post) describing an action's blast radius. The gateway sanitizes it once at the producer boundary, the core view model renders a Scope metadata row so Slack/Discord/Google Chat cards show it unchanged, shared text builders cover Telegram/WhatsApp/Signal/iMessage/Matrix, and the durable presentation carries it additively for operator surfaces. Scope is display-only, never authorization; missing scope keeps today's cards. * fix(approvals): emit native ApprovalScope union and clamp recipient previews Name the three scope variants as registered protocol schemas so the Swift generator emits the ApprovalScope discriminated union the presentation structs reference, and commit the regenerated GatewayModels.swift. Clamp recipient previews to the declared recipientCount at the sanitize boundary so a count of 1 with 2 previews can no longer render inconsistently. Addresses both ClawSweeper findings on #130116. * refactor(approvals): extract text sanitizer to break the exec-approvals import cycle check:architecture flagged approval-scope joining the exec-approvals SCC through exec-approval-command-display. Move the self-contained display sanitizer into a leaf module (exec-approval-text-sanitize) with no exec-approvals imports and migrate all sanitize importers; command-display keeps only the payload-typed command/preview resolver. * chore(plugin-sdk): ratchet public surface budgets down after sanitizer extraction The approval display sanitizers left the publicly reachable SDK graph when they moved to the exec-approval-text-sanitize leaf: exports 4343 -> 4338, callable exports 2582 -> 2578. Shrink-only budget pin. --- .../OpenClawProtocol/GatewayModels.swift | 115 ++++++++++ docs/plugins/plugin-permission-requests.md | 59 +++++ docs/tools/exec-approvals.md | 17 ++ extensions/imessage/src/approval-native.ts | 1 + .../matrix/src/approval-handler.runtime.ts | 2 + .../src/approval-handler.runtime.test.ts | 10 + .../src/approval-handler.runtime.test.ts | 45 ++++ .../telegram/src/approval-handler.runtime.ts | 1 + .../telegram/src/exec-approval-forwarding.ts | 1 + .../src/approvals-validators.test.ts | 32 +++ .../gateway-protocol/src/public-schema.ts | 1 + .../gateway-protocol/src/schema/approvals.ts | 39 ++++ .../src/schema/exec-approvals.ts | 3 +- .../src/schema/plugin-approvals.ts | 3 +- .../protocol-schema-fragment-approvals.ts | 4 + scripts/plugin-sdk-surface-report.mts | 9 +- .../agent-tools.before-tool-call.approval.ts | 3 + ...ols.before-tool-call.embedded-mode.test.ts | 6 + .../cli-runner/cli-native-tool-approval.ts | 2 +- src/cli/exec-policy-cli.ts | 2 +- src/gateway/node-invoke-plugin-policy.ts | 4 +- src/gateway/server-methods/exec-approval.ts | 9 +- .../plugin-approval.scope.test.ts | 88 ++++++++ src/gateway/server-methods/plugin-approval.ts | 7 +- src/infra/approval-presentation.test.ts | 34 +++ src/infra/approval-presentation.ts | 9 +- src/infra/approval-scope.test.ts | 90 ++++++++ src/infra/approval-scope.ts | 63 ++++++ src/infra/approval-view-model.test.ts | 34 +++ src/infra/approval-view-model.ts | 9 + src/infra/approval-view-model.types.ts | 3 + .../exec-approval-command-display.test.ts | 4 +- src/infra/exec-approval-command-display.ts | 202 +----------------- src/infra/exec-approval-forwarder.ts | 6 +- src/infra/exec-approval-reply.test.ts | 6 +- src/infra/exec-approval-reply.ts | 5 + src/infra/exec-approval-text-sanitize.ts | 201 +++++++++++++++++ src/infra/exec-approvals-core.ts | 3 + src/infra/plugin-approvals.ts | 6 + .../approval-reaction-runtime.test.ts | 15 +- src/plugin-sdk/approval-reaction-runtime.ts | 9 + src/plugin-sdk/approval-runtime.ts | 1 + src/plugins/hook-before-tool-call-result.ts | 3 + src/plugins/plugin-registration.types.ts | 2 + 44 files changed, 945 insertions(+), 223 deletions(-) create mode 100644 src/gateway/server-methods/plugin-approval.scope.test.ts create mode 100644 src/infra/approval-scope.test.ts create mode 100644 src/infra/approval-scope.ts create mode 100644 src/infra/exec-approval-text-sanitize.ts diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 2bc3f898eadc..689ffa3bbe53 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -17619,6 +17619,71 @@ public struct TerminalExitEvent: Codable, Sendable { } } +public struct MessageSendApprovalScope: Codable, Sendable { + public let kind: String + public let target: String + public let recipientcount: Int + public let recipients: [String]? + public let audience: AnyCodable? + + public init( + kind: String, + target: String, + recipientcount: Int, + recipients: [String]? = nil, + audience: AnyCodable? = nil) + { + self.kind = kind + self.target = target + self.recipientcount = recipientcount + self.recipients = recipients + self.audience = audience + } + + private enum CodingKeys: String, CodingKey { + case kind + case target + case recipientcount = "recipientCount" + case recipients + case audience + } +} + +public struct PaymentApprovalScope: Codable, Sendable { + public let kind: String + public let amount: String + public let currency: String + public let target: String + + public init( + kind: String, + amount: String, + currency: String, + target: String) + { + self.kind = kind + self.amount = amount + self.currency = currency + self.target = target + } +} + +public struct ExternalPostApprovalScope: Codable, Sendable { + public let kind: String + public let target: String + public let visibility: AnyCodable + + public init( + kind: String, + target: String, + visibility: AnyCodable) + { + self.kind = kind + self.target = target + self.visibility = visibility + } +} + public struct ExecApprovalPresentation: Codable, Sendable { public let kind: String public let commandtext: String @@ -17627,6 +17692,7 @@ public struct ExecApprovalPresentation: Codable, Sendable { public let host: AnyCodable? public let nodeid: AnyCodable? public let agentid: AnyCodable? + public let scope: ApprovalScope? public let alloweddecisions: [ApprovalDecision] public init( @@ -17637,6 +17703,7 @@ public struct ExecApprovalPresentation: Codable, Sendable { host: AnyCodable? = nil, nodeid: AnyCodable? = nil, agentid: AnyCodable? = nil, + scope: ApprovalScope? = nil, alloweddecisions: [ApprovalDecision]) { self.kind = kind @@ -17646,6 +17713,7 @@ public struct ExecApprovalPresentation: Codable, Sendable { self.host = host self.nodeid = nodeid self.agentid = agentid + self.scope = scope self.alloweddecisions = alloweddecisions } @@ -17657,6 +17725,7 @@ public struct ExecApprovalPresentation: Codable, Sendable { case host case nodeid = "nodeId" case agentid = "agentId" + case scope case alloweddecisions = "allowedDecisions" } } @@ -17670,6 +17739,7 @@ public struct PluginApprovalPresentation: Codable, Sendable { public let pluginid: AnyCodable? public let toolname: AnyCodable? public let agentid: AnyCodable? + public let scope: ApprovalScope? public let alloweddecisions: [ApprovalDecision] public init( @@ -17681,6 +17751,7 @@ public struct PluginApprovalPresentation: Codable, Sendable { pluginid: AnyCodable? = nil, toolname: AnyCodable? = nil, agentid: AnyCodable? = nil, + scope: ApprovalScope? = nil, alloweddecisions: [ApprovalDecision]) { self.kind = kind @@ -17691,6 +17762,7 @@ public struct PluginApprovalPresentation: Codable, Sendable { self.pluginid = pluginid self.toolname = toolname self.agentid = agentid + self.scope = scope self.alloweddecisions = alloweddecisions } @@ -17703,6 +17775,7 @@ public struct PluginApprovalPresentation: Codable, Sendable { case pluginid = "pluginId" case toolname = "toolName" case agentid = "agentId" + case scope case alloweddecisions = "allowedDecisions" } } @@ -18321,6 +18394,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { public let security: AnyCodable? public let ask: AnyCodable? public let warningtext: AnyCodable? + public let scope: ApprovalScope? public let unavailabledecisions: [String]? public let commandspans: [[String: AnyCodable]]? public let agentid: AnyCodable? @@ -18352,6 +18426,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { security: AnyCodable? = nil, ask: AnyCodable? = nil, warningtext: AnyCodable? = nil, + scope: ApprovalScope? = nil, unavailabledecisions: [String]? = nil, commandspans: [[String: AnyCodable]]? = nil, agentid: AnyCodable? = nil, @@ -18382,6 +18457,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { self.security = security self.ask = ask self.warningtext = warningtext + self.scope = scope self.unavailabledecisions = unavailabledecisions self.commandspans = commandspans self.agentid = agentid @@ -18414,6 +18490,7 @@ public struct ExecApprovalRequestParams: Codable, Sendable { case security case ask case warningtext = "warningText" + case scope case unavailabledecisions = "unavailableDecisions" case commandspans = "commandSpans" case agentid = "agentId" @@ -18778,6 +18855,7 @@ public struct PluginApprovalRequestParams: Codable, Sendable { public let description: String public let detail: String? public let severity: String? + public let scope: ApprovalScope? public let toolname: String? public let toolcallid: String? public let alloweddecisions: [String]? @@ -18797,6 +18875,7 @@ public struct PluginApprovalRequestParams: Codable, Sendable { description: String, detail: String? = nil, severity: String? = nil, + scope: ApprovalScope? = nil, toolname: String? = nil, toolcallid: String? = nil, alloweddecisions: [String]? = nil, @@ -18815,6 +18894,7 @@ public struct PluginApprovalRequestParams: Codable, Sendable { self.description = description self.detail = detail self.severity = severity + self.scope = scope self.toolname = toolname self.toolcallid = toolcallid self.alloweddecisions = alloweddecisions @@ -18835,6 +18915,7 @@ public struct PluginApprovalRequestParams: Codable, Sendable { case description case detail case severity + case scope case toolname = "toolName" case toolcallid = "toolCallId" case alloweddecisions = "allowedDecisions" @@ -21655,6 +21736,40 @@ public enum ToolsGitHubAuthorizePollResult: Codable, Sendable { } } +public enum ApprovalScope: Codable, Sendable { + case messageSend(MessageSendApprovalScope) + case payment(PaymentApprovalScope) + case externalPost(ExternalPostApprovalScope) + + private enum CodingKeys: String, CodingKey { + case discriminator = "kind" + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let discriminator = try container.decode(String.self, forKey: .discriminator) + switch discriminator { + case "message-send": self = try .messageSend(MessageSendApprovalScope(from: decoder)) + case "payment": self = try .payment(PaymentApprovalScope(from: decoder)) + case "external-post": self = try .externalPost(ExternalPostApprovalScope(from: decoder)) + default: + throw DecodingError.dataCorruptedError( + forKey: .discriminator, + in: container, + debugDescription: "Unknown ApprovalScope discriminator value" + ) + } + } + + public func encode(to encoder: Encoder) throws { + switch self { + case .messageSend(let value): try value.encode(to: encoder) + case .payment(let value): try value.encode(to: encoder) + case .externalPost(let value): try value.encode(to: encoder) + } + } +} + public enum ApprovalPresentation: Codable, Sendable { case exec(ExecApprovalPresentation) case plugin(PluginApprovalPresentation) diff --git a/docs/plugins/plugin-permission-requests.md b/docs/plugins/plugin-permission-requests.md index 26397ffacbdc..b26bd6e31293 100644 --- a/docs/plugins/plugin-permission-requests.md +++ b/docs/plugins/plugin-permission-requests.md @@ -88,6 +88,65 @@ Write prompt text for the person who will approve the action: - `timeoutMs` defaults to 120000 (2 minutes) and is capped at 600000 (10 minutes) regardless of the requested value. +## Declare approval scope + +Set `requireApproval.scope` when your plugin knows the consequences of an +operation. Scope is typed, optional, and display-only: it helps reviewers +understand the action but never grants permission or changes the approval +decision. The plugin declaring the approval supplies these facts; channels never +infer scope from commands, titles, or message text. + +For an email to three external recipients, include the destination, total +recipient count, an optional preview, and the audience: + +```typescript +requireApproval: { + title: "Send customer update", + scope: { + kind: "message-send", + target: "email", + recipientCount: 3, + recipients: ["alice@example.com", "bob@example.com"], + audience: "external", + }, +} +``` + +For a payment, provide the exact decimal amount as a string, its currency, and +the payee or payment system: + +```typescript +requireApproval: { + title: "Pay invoice", + scope: { + kind: "payment", + amount: "49.99", + currency: "EUR", + target: "Stripe", + }, +} +``` + +For an external post, identify its destination and declare its visibility: + +```typescript +requireApproval: { + title: "Publish announcement", + scope: { + kind: "external-post", + target: "github", + visibility: "public", + }, +} +``` + +Message audiences can be `internal` or `external`; external-post visibility can +be `public` or `restricted`. Recipient previews contain at most five identities. +All strings are sanitized and bounded before display: targets and recipient +identities are limited to 128 characters, payment amounts to 40, and currencies +to 12. If sanitization would exceed a bound, OpenClaw omits the scope while +preserving the normal approval prompt. + ## Decision behavior OpenClaw creates a pending approval with a `plugin:` ID, delivers it to the diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 484b8c8b3cdf..37e7f29e61a0 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -511,6 +511,23 @@ context when forwarding approved `system.run` requests: - Once approved, the final forwarded `system.run` call reuses the stored plan instead of trusting later caller edits. - If the caller changes `command`, `rawCommand`, `cwd`, `agentId`, or `sessionKey` after the approval request was created, the gateway rejects the forwarded run as an approval mismatch. +## Approval scope summaries + +An approval owner can attach a typed, display-only scope describing the action's +blast radius. OpenClaw renders the sanitized summary on channel approval cards +and includes the bounded scope in the safe approval presentation available to +Control UI clients. Scope never grants authorization or changes approval policy. + +- `message-send`: destination, recipient count, optional recipient preview, and + whether the audience is internal or external. +- `payment`: exact decimal amount, currency, and payee or payment system. +- `external-post`: destination and whether the post is public or restricted. + +For example, an email approval might show `Send to 3 recipients via email +(external): alice@example.com, bob@example.com, +1 more`. Owners supply these +facts; channels never infer them from commands or message text. Without a +declared scope, approval cards render exactly as before. + ## System events and denials Exec lifecycle posts an `Exec finished` system message to the agent's diff --git a/extensions/imessage/src/approval-native.ts b/extensions/imessage/src/approval-native.ts index a5b9c2586cac..fc7bb7233a5b 100644 --- a/extensions/imessage/src/approval-native.ts +++ b/extensions/imessage/src/approval-native.ts @@ -225,6 +225,7 @@ function buildIMessageExecPendingPayload(params: { request: ExecApprovalRequest; cwd: params.request.request.cwd ?? undefined, host: params.request.request.host === "node" ? "node" : "gateway", nodeId: params.request.request.nodeId ?? undefined, + scope: params.request.request.scope ?? undefined, sessionKey: params.request.request.sessionKey ?? null, expiresAtMs: params.request.expiresAtMs, nowMs: params.nowMs, diff --git a/extensions/matrix/src/approval-handler.runtime.ts b/extensions/matrix/src/approval-handler.runtime.ts index 8e6a0b2fe92b..714a68580d75 100644 --- a/extensions/matrix/src/approval-handler.runtime.ts +++ b/extensions/matrix/src/approval-handler.runtime.ts @@ -308,6 +308,7 @@ function buildPendingApprovalContent(params: { toolName: params.view.toolName ?? undefined, pluginId: params.view.pluginId ?? undefined, agentId: params.view.agentId ?? undefined, + scope: params.view.scope ?? undefined, }, createdAtMs: 0, expiresAtMs: params.view.expiresAtMs, @@ -326,6 +327,7 @@ function buildPendingApprovalContent(params: { cwd: params.view.cwd ?? undefined, host: params.view.host === "node" ? "node" : "gateway", nodeId: params.view.nodeId ?? undefined, + scope: params.view.scope ?? undefined, sessionKey: params.view.sessionKey ?? undefined, expiresAtMs: params.view.expiresAtMs, nowMs: params.nowMs, diff --git a/extensions/slack/src/approval-handler.runtime.test.ts b/extensions/slack/src/approval-handler.runtime.test.ts index 22acef6806e3..a41484a03744 100644 --- a/extensions/slack/src/approval-handler.runtime.test.ts +++ b/extensions/slack/src/approval-handler.runtime.test.ts @@ -394,6 +394,10 @@ describe("slackApprovalNativeRuntime", () => { metadata: [ { label: "Severity", value: "Warning" }, { label: "Plugin", value: "computer-use" }, + { + label: "Scope", + value: "Send to 3 recipients via email (external): alice@example.com, +2 more", + }, ], decisions: ["allow-once", "allow-always", "deny"], }); @@ -404,6 +408,12 @@ describe("slackApprovalNativeRuntime", () => { ); expect(payload.text).toContain("Share screen with Computer Use"); expect(payload.text).toContain("*Approval ID:* plugin:req-1"); + expect(payload.text).toContain( + "*Scope:* Send to 3 recipients via email (external): alice@example.com, +2 more", + ); + expect(readMrkdwnTexts(payload.blocks)).toContain( + "*Scope:* Send to 3 recipients via email (external): alice@example.com, +2 more", + ); expect(payload.text).not.toContain("*Command*"); const actionsBlock = findSlackActionsBlock( payload.blocks as Array<{ type?: string; elements?: unknown[] }>, diff --git a/extensions/telegram/src/approval-handler.runtime.test.ts b/extensions/telegram/src/approval-handler.runtime.test.ts index f9ed41179e34..c504f6859f24 100644 --- a/extensions/telegram/src/approval-handler.runtime.test.ts +++ b/extensions/telegram/src/approval-handler.runtime.test.ts @@ -96,6 +96,51 @@ describe("telegramApprovalNativeRuntime", () => { "tga1:e:o:req-1", "tga1:e:d:req-1", ]); + expect(payload.text).not.toContain("Scope:"); + }); + + it("renders owner-declared plugin approval scope in pending text", async () => { + const scope = { + kind: "message-send" as const, + target: "email", + recipientCount: 3, + recipients: ["alice@example.com"], + audience: "external" as const, + }; + const payload = (await telegramApprovalNativeRuntime.presentation.buildPendingPayload({ + cfg: {} as never, + accountId: "default", + context: { token: "tg-token" }, + request: { + approvalKind: "plugin", + id: "plugin:req-1", + request: { + title: "Send email", + description: "Deliver the requested announcement.", + scope, + }, + createdAtMs: 0, + expiresAtMs: 60_000, + }, + approvalKind: "plugin", + nowMs: 0, + view: { + approvalKind: "plugin", + phase: "pending", + approvalId: "plugin:req-1", + title: "Send email", + description: "Deliver the requested announcement.", + severity: "warning", + scope, + metadata: [], + actions: [], + expiresAtMs: 60_000, + }, + })) as TelegramPayload; + + expect(payload.text).toContain( + "Scope: Send to 3 recipients via email (external): alice@example.com, +2 more", + ); }); it("renders resolved and expired events as visible terminal receipts", async () => { diff --git a/extensions/telegram/src/approval-handler.runtime.ts b/extensions/telegram/src/approval-handler.runtime.ts index 0a7ce176d702..5df21e8e5d32 100644 --- a/extensions/telegram/src/approval-handler.runtime.ts +++ b/extensions/telegram/src/approval-handler.runtime.ts @@ -101,6 +101,7 @@ function buildPendingPayload(params: { params.view.approvalKind === "exec" && params.view.host === "node" ? "node" : "gateway", nodeId: params.view.approvalKind === "exec" ? (params.view.nodeId ?? undefined) : undefined, + scope: params.view.approvalKind === "exec" ? (params.view.scope ?? undefined) : undefined, allowedDecisions: params.view.actions.map((action) => action.decision), expiresAtMs: params.request.expiresAtMs, nowMs: params.nowMs, diff --git a/extensions/telegram/src/exec-approval-forwarding.ts b/extensions/telegram/src/exec-approval-forwarding.ts index 7bd604a49ab7..6bb6ba4540ff 100644 --- a/extensions/telegram/src/exec-approval-forwarding.ts +++ b/extensions/telegram/src/exec-approval-forwarding.ts @@ -40,6 +40,7 @@ export function buildTelegramExecApprovalPendingPayload(params: { cwd: params.request.request.cwd ?? undefined, host: params.request.request.host === "node" ? "node" : "gateway", nodeId: params.request.request.nodeId ?? undefined, + scope: params.request.request.scope ?? undefined, allowedDecisions: resolveExecApprovalRequestAllowedDecisions(params.request.request), expiresAtMs: params.request.expiresAtMs, nowMs: params.nowMs, diff --git a/packages/gateway-protocol/src/approvals-validators.test.ts b/packages/gateway-protocol/src/approvals-validators.test.ts index 802d14cf6b26..ced27a92f853 100644 --- a/packages/gateway-protocol/src/approvals-validators.test.ts +++ b/packages/gateway-protocol/src/approvals-validators.test.ts @@ -81,6 +81,38 @@ describe("unified approval protocol validators", () => { } }); + it("accepts bounded owner-declared approval scopes and rejects unknown scope fields", () => { + const scopes = [ + { + kind: "message-send", + target: "email", + recipientCount: 3, + recipients: ["alice@example.com", "bob@example.com"], + audience: "external", + }, + { kind: "payment", amount: "49.99", currency: "EUR", target: "Stripe" }, + { kind: "external-post", target: "github", visibility: "public" }, + ] as const; + + for (const scope of scopes) { + expect(validateApprovalPresentation({ ...execPresentation, scope })).toBe(true); + expect(validateApprovalPresentation({ ...pluginPresentation, scope })).toBe(true); + expect( + validateApprovalPresentation({ ...pluginPresentation, scope: { ...scope, extra: true } }), + ).toBe(false); + } + + expect(validateApprovalPresentation({ ...systemAgentPresentation, scope: scopes[0] })).toBe( + false, + ); + expect( + validateApprovalPresentation({ + ...pluginPresentation, + scope: { ...scopes[0], recipients: Array(6).fill("person@example.com") }, + }), + ).toBe(false); + }); + it("keeps deny available on every presentation and resolve request", () => { expect( validateApprovalPresentation({ diff --git a/packages/gateway-protocol/src/public-schema.ts b/packages/gateway-protocol/src/public-schema.ts index 3f91214312d8..c7300e6917cc 100644 --- a/packages/gateway-protocol/src/public-schema.ts +++ b/packages/gateway-protocol/src/public-schema.ts @@ -601,6 +601,7 @@ export { ApprovalAllowDecisionSchema, ApprovalTerminalReasonSchema, PluginApprovalSeveritySchema, + ApprovalScopeSchema, ExecApprovalPresentationSchema, PluginApprovalPresentationSchema, ApprovalPresentationSchema, diff --git a/packages/gateway-protocol/src/schema/approvals.ts b/packages/gateway-protocol/src/schema/approvals.ts index bc3dddf90773..e810d7f650c3 100644 --- a/packages/gateway-protocol/src/schema/approvals.ts +++ b/packages/gateway-protocol/src/schema/approvals.ts @@ -72,6 +72,42 @@ export const PluginApprovalSeveritySchema = Type.Union([ Type.Literal("critical"), ]); +/** Message/email delivery blast radius declared by the approval owner. */ +export const MessageSendApprovalScopeSchema = closedObject({ + kind: Type.Literal("message-send"), + target: Type.String({ minLength: 1, maxLength: 128 }), + recipientCount: Type.Integer({ minimum: 1, maximum: 1_000_000 }), + recipients: Type.Optional( + Type.Array(Type.String({ minLength: 1, maxLength: 128 }), { maxItems: 5 }), + ), + audience: Type.Optional(Type.Union([Type.Literal("internal"), Type.Literal("external")])), +}); + +/** Payment blast radius declared by the approval owner. */ +export const PaymentApprovalScopeSchema = closedObject({ + kind: Type.Literal("payment"), + amount: Type.String({ minLength: 1, maxLength: 40 }), + currency: Type.String({ minLength: 1, maxLength: 12 }), + target: Type.String({ minLength: 1, maxLength: 128 }), +}); + +/** External publication blast radius declared by the approval owner. */ +export const ExternalPostApprovalScopeSchema = closedObject({ + kind: Type.Literal("external-post"), + target: Type.String({ minLength: 1, maxLength: 128 }), + visibility: Type.Union([Type.Literal("public"), Type.Literal("restricted")]), +}); + +/** + * Owner-declared blast-radius facts for a pending approval. Variants are + * named schemas so native protocol generators emit the discriminated union. + */ +export const ApprovalScopeSchema = Type.Union([ + MessageSendApprovalScopeSchema, + PaymentApprovalScopeSchema, + ExternalPostApprovalScopeSchema, +]); + const ApprovalAllowedDecisionsSchema = Type.Array(ApprovalDecisionSchema, { minItems: 1, maxItems: 3, @@ -96,6 +132,7 @@ export const ExecApprovalPresentationSchema = Type.Object( host: Type.Optional(Type.Union([Type.String(), Type.Null()])), nodeId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), agentId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), + scope: Type.Optional(ApprovalScopeSchema), allowedDecisions: ApprovalAllowedDecisionsSchema, }, { @@ -115,6 +152,7 @@ export const PluginApprovalPresentationSchema = closedObject({ pluginId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), toolName: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), agentId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), + scope: Type.Optional(ApprovalScopeSchema), allowedDecisions: ApprovalAllowedDecisionsSchema, }); @@ -312,6 +350,7 @@ export type ApprovalDecision = Static; export type ApprovalAllowDecision = Static; export type ApprovalTerminalReason = Static; export type PluginApprovalSeverity = Static; +export type ApprovalScope = Static; export type ExecApprovalPresentation = Static; export type PluginApprovalPresentation = Static; export type SystemAgentApprovalPresentation = Static; diff --git a/packages/gateway-protocol/src/schema/exec-approvals.ts b/packages/gateway-protocol/src/schema/exec-approvals.ts index 7d7df43c9ee4..a8706d5976fd 100644 --- a/packages/gateway-protocol/src/schema/exec-approvals.ts +++ b/packages/gateway-protocol/src/schema/exec-approvals.ts @@ -1,7 +1,7 @@ import type { Static } from "typebox"; // Gateway Protocol schema module defines protocol validation shapes. import { Type } from "typebox"; -import { ApprovalChannelReviewerSchema } from "./approvals.js"; +import { ApprovalChannelReviewerSchema, ApprovalScopeSchema } from "./approvals.js"; import { closedObject } from "./closed-object.js"; import { NonEmptyString } from "./primitives.js"; @@ -266,6 +266,7 @@ export const ExecApprovalRequestParamsSchema = closedObject({ security: Type.Optional(Type.Union([Type.String(), Type.Null()])), ask: Type.Optional(Type.Union([Type.String(), Type.Null()])), warningText: Type.Optional(Type.Union([Type.String(), Type.Null()])), + scope: Type.Optional(ApprovalScopeSchema), unavailableDecisions: Type.Optional( Type.Array(Type.String({ enum: ["allow-always"] }), { minItems: 1, diff --git a/packages/gateway-protocol/src/schema/plugin-approvals.ts b/packages/gateway-protocol/src/schema/plugin-approvals.ts index a4226c284da1..d840c59c2d0e 100644 --- a/packages/gateway-protocol/src/schema/plugin-approvals.ts +++ b/packages/gateway-protocol/src/schema/plugin-approvals.ts @@ -1,7 +1,7 @@ import type { Static } from "typebox"; // Gateway Protocol schema module defines protocol validation shapes. import { Type } from "typebox"; -import { ApprovalChannelReviewerSchema } from "./approvals.js"; +import { ApprovalChannelReviewerSchema, ApprovalScopeSchema } from "./approvals.js"; import { closedObject } from "./closed-object.js"; import { NonEmptyString } from "./primitives.js"; @@ -29,6 +29,7 @@ export const PluginApprovalRequestParamsSchema = closedObject({ }), ), severity: Type.Optional(Type.String({ enum: ["info", "warning", "critical"] })), + scope: Type.Optional(ApprovalScopeSchema), toolName: Type.Optional(Type.String()), toolCallId: Type.Optional(Type.String()), allowedDecisions: Type.Optional( diff --git a/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts b/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts index 4347e97b2ec7..2a4d307c6725 100644 --- a/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts +++ b/packages/gateway-protocol/src/schema/protocol-schema-fragment-approvals.ts @@ -11,6 +11,10 @@ export const ApprovalProtocolSchemas = { ApprovalExpiredReason: approvals.ApprovalExpiredReasonSchema, ApprovalCancelledReason: approvals.ApprovalCancelledReasonSchema, PluginApprovalSeverity: approvals.PluginApprovalSeveritySchema, + MessageSendApprovalScope: approvals.MessageSendApprovalScopeSchema, + PaymentApprovalScope: approvals.PaymentApprovalScopeSchema, + ExternalPostApprovalScope: approvals.ExternalPostApprovalScopeSchema, + ApprovalScope: approvals.ApprovalScopeSchema, ExecApprovalPresentation: approvals.ExecApprovalPresentationSchema, PluginApprovalPresentation: approvals.PluginApprovalPresentationSchema, SystemAgentApprovalPresentation: approvals.SystemAgentApprovalPresentationSchema, diff --git a/scripts/plugin-sdk-surface-report.mts b/scripts/plugin-sdk-surface-report.mts index 9c596d0a389b..7c0dd489c3fe 100644 --- a/scripts/plugin-sdk-surface-report.mts +++ b/scripts/plugin-sdk-surface-report.mts @@ -313,7 +313,10 @@ export function readPluginSdkSurfaceBudgets(env: NodeJS.ProcessEnv = process.env // +2: restore shipped channel setup helpers until stable packages migrate. // +1: canonical untrusted audio-transcript formatter for channel plugins. // +2: embedded foreground prompt context builder and its public context type. - 4342, + // +1: typed owner-declared approval-scope contract for plugin-authored approvals. + // -5: approval display sanitizers moved to a non-public leaf module + // (exec-approval-text-sanitize) to break the exec-approvals cycle. + 4338, env, ), publicFunctionExports: readPluginSdkSurfaceBudgetEnv( @@ -406,7 +409,9 @@ export function readPluginSdkSurfaceBudgets(env: NodeJS.ProcessEnv = process.env // +2: restore shipped channel setup helpers until stable packages migrate. // +1: canonical untrusted audio-transcript formatter for channel plugins. // +1: embedded foreground prompt context builder. - 2582, + // -4: approval display sanitizers moved to a non-public leaf module + // (exec-approval-text-sanitize) to break the exec-approvals cycle. + 2578, env, ), publicDeprecatedExports: readPluginSdkSurfaceBudgetEnv( diff --git a/src/agents/agent-tools.before-tool-call.approval.ts b/src/agents/agent-tools.before-tool-call.approval.ts index 887cbc6bf956..3a57c61b92b9 100644 --- a/src/agents/agent-tools.before-tool-call.approval.ts +++ b/src/agents/agent-tools.before-tool-call.approval.ts @@ -5,6 +5,7 @@ */ import { addTimerTimeoutGraceMs } from "@openclaw/normalization-core/number-coercion"; import { GatewayClientRequestError } from "../gateway/client.js"; +import { sanitizeApprovalScope } from "../infra/approval-scope.js"; import { isEmbeddedMode } from "../infra/embedded-mode.js"; import { getEmbeddedPluginApprovalBroker } from "../infra/embedded-plugin-approval-broker.js"; import { formatErrorMessage } from "../infra/errors.js"; @@ -201,6 +202,7 @@ async function requestPluginToolApproval(params: { pluginId: approval.pluginId, title: approval.title, description: approval.description, + ...(approval.scope ? { scope: sanitizeApprovalScope(approval.scope) } : {}), severity: approval.severity, allowedDecisions: approval.allowedDecisions, toolName: params.toolName, @@ -288,6 +290,7 @@ async function requestPluginToolApproval(params: { { title: approval.title, description: approval.description, + ...(approval.scope ? { scope: approval.scope } : {}), severity: approval.severity, allowedDecisions: approval.allowedDecisions, toolName: params.toolName, diff --git a/src/agents/agent-tools.before-tool-call.embedded-mode.test.ts b/src/agents/agent-tools.before-tool-call.embedded-mode.test.ts index 357b48d7d295..e2ba461e0864 100644 --- a/src/agents/agent-tools.before-tool-call.embedded-mode.test.ts +++ b/src/agents/agent-tools.before-tool-call.embedded-mode.test.ts @@ -235,6 +235,7 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => { pluginId: "test-plugin", title: "Needs approval", description: "Test approval request", + scope: { kind: "external-post", target: "git‮hub", visibility: "public" }, severity: "info", timeoutBehavior: "allow", onResolution, @@ -251,6 +252,11 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => { await vi.waitFor(() => { expect(broker.listPending()).toHaveLength(1); }); + expect(broker.listPending()[0]?.request.scope).toEqual({ + kind: "external-post", + target: "git\\u{202E}hub", + visibility: "public", + }); broker.stop(new Error("local TUI stopped")); diff --git a/src/agents/cli-runner/cli-native-tool-approval.ts b/src/agents/cli-runner/cli-native-tool-approval.ts index 06e6aecf064c..94185b84cf8f 100644 --- a/src/agents/cli-runner/cli-native-tool-approval.ts +++ b/src/agents/cli-runner/cli-native-tool-approval.ts @@ -1,5 +1,5 @@ import { addTimerTimeoutGraceMs } from "@openclaw/normalization-core/number-coercion"; -import { sanitizeExecApprovalWarningTextWithStatus } from "../../infra/exec-approval-command-display.js"; +import { sanitizeExecApprovalWarningTextWithStatus } from "../../infra/exec-approval-text-sanitize.js"; import type { ExecAsk, ExecSecurity } from "../../infra/exec-approvals.js"; import { DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS, diff --git a/src/cli/exec-policy-cli.ts b/src/cli/exec-policy-cli.ts index 17d4c70ee6d2..88421c2f5969 100644 --- a/src/cli/exec-policy-cli.ts +++ b/src/cli/exec-policy-cli.ts @@ -6,7 +6,7 @@ import { getTerminalTableWidth, renderTable } from "../../packages/terminal-core import { isRich, theme } from "../../packages/terminal-core/src/theme.js"; import { readConfigFileSnapshot, replaceConfigFile } from "../config/config.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { sanitizeExecApprovalDisplayText } from "../infra/exec-approval-command-display.js"; +import { sanitizeExecApprovalDisplayText } from "../infra/exec-approval-text-sanitize.js"; import { collectExecPolicyScopeSnapshots, SESSION_EXEC_OVERRIDES_NOTE, diff --git a/src/gateway/node-invoke-plugin-policy.ts b/src/gateway/node-invoke-plugin-policy.ts index f2bbb809adb6..264791e15b6d 100644 --- a/src/gateway/node-invoke-plugin-policy.ts +++ b/src/gateway/node-invoke-plugin-policy.ts @@ -4,10 +4,11 @@ import { randomUUID } from "node:crypto"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; import { recordRuntimeActionDecision } from "../audit/runtime-action-decision.js"; +import { sanitizeApprovalScope } from "../infra/approval-scope.js"; import { sanitizeExecApprovalDisplayText, sanitizeExecApprovalWarningText, -} from "../infra/exec-approval-command-display.js"; +} from "../infra/exec-approval-text-sanitize.js"; import { resolveCanonicalPluginApprovalRequestAllowedDecisions } from "../infra/plugin-approval-canonical-decisions.js"; import type { PluginApprovalRequestPayload } from "../infra/plugin-approvals.js"; import { resolvePluginApprovalTimeoutMs } from "../infra/plugin-approvals.js"; @@ -145,6 +146,7 @@ function createApprovalRuntime(params: { sanitizeExecApprovalWarningText(normalizeOptionalString(input.description) ?? ""), 256, ), + scope: input.scope ? sanitizeApprovalScope(input.scope) : null, severity: input.severity ?? "warning", ...(input.allowedDecisions === undefined ? {} diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index c31a6528a571..55cf2f6eb3dd 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -9,15 +9,16 @@ import { validateExecApprovalResolveParams, } from "../../../packages/gateway-protocol/src/index.js"; import { resolveExecCommandHighlighting } from "../../config/exec-command-highlighting.js"; +import { sanitizeApprovalScope, type ApprovalScope } from "../../infra/approval-scope.js"; import { resolveCommandAnalysisSummaryForDisplay } from "../../infra/command-analysis/explain.js"; import { lookupCronRunExecSource } from "../../infra/cron-run-exec-source.js"; +import { resolveExecApprovalCommandDisplay } from "../../infra/exec-approval-command-display.js"; +import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js"; import { - resolveExecApprovalCommandDisplay, sanitizeExecApprovalDisplayText, sanitizeExecApprovalDisplayTextWithStatus, sanitizeExecApprovalWarningText, -} from "../../infra/exec-approval-command-display.js"; -import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js"; +} from "../../infra/exec-approval-text-sanitize.js"; import { normalizeExecAsk, normalizeExecSecurity } from "../../infra/exec-approvals-core.js"; import { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS, @@ -170,6 +171,7 @@ export function createExecApprovalHandlers( security?: string; ask?: string; warningText?: string | null; + scope?: ApprovalScope; unavailableDecisions?: string[]; commandSpans?: { startIndex: number; @@ -367,6 +369,7 @@ export function createExecApprovalHandlers( security: normalizeExecSecurity(p.security) ?? null, ask: normalizeExecAsk(p.ask) ?? null, warningText: warningText ? sanitizeExecApprovalWarningText(warningText) : null, + scope: p.scope ? sanitizeApprovalScope(p.scope) : null, commandAnalysis, commandSpans, unavailableDecisions: unavailableDecisions.length > 0 ? unavailableDecisions : undefined, diff --git a/src/gateway/server-methods/plugin-approval.scope.test.ts b/src/gateway/server-methods/plugin-approval.scope.test.ts new file mode 100644 index 000000000000..052524ecef12 --- /dev/null +++ b/src/gateway/server-methods/plugin-approval.scope.test.ts @@ -0,0 +1,88 @@ +import { expectDefined } from "@openclaw/normalization-core"; +import { describe, expect, it, vi } from "vitest"; +import type { PluginApprovalRequestPayload } from "../../infra/plugin-approvals.js"; +import { ExecApprovalManager } from "../exec-approval-manager.js"; +import { createPluginApprovalHandlers } from "./plugin-approval.js"; +import type { GatewayRequestHandlerOptions } from "./types.js"; + +function createApprovalScopeRequest(scope: unknown) { + const manager = new ExecApprovalManager({ approvalKind: "plugin" }); + const respond = vi.fn(); + const params = { + title: "Sensitive action", + description: "Review the action", + scope, + twoPhase: true, + }; + const options = { + req: { method: "plugin.approval.request", params, id: "request" }, + params, + respond, + client: { connId: "reviewer", connect: { client: { id: "reviewer" } } }, + context: { + broadcast: vi.fn(), + logGateway: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() }, + hasExecApprovalClients: () => true, + }, + } as unknown as GatewayRequestHandlerOptions; + const handler = expectDefined( + createPluginApprovalHandlers(manager)["plugin.approval.request"], + "plugin approval request handler", + ); + return { manager, respond, handler, options }; +} + +describe("plugin approval request scopes", () => { + it("sanitizes owner-declared scope before storing or broadcasting the approval", async () => { + const { manager, handler, options } = createApprovalScopeRequest({ + kind: "message-send", + target: "email\u202Esystem", + recipientCount: 3, + recipients: ["alice\u200B@example.com", "bob@example.com"], + audience: "external", + }); + const pending = handler(options); + await vi.waitFor(() => expect(manager.listPendingRecords()).toHaveLength(1)); + const record = expectDefined(manager.listPendingRecords()[0], "pending plugin approval"); + + expect(record.request.scope).toEqual({ + kind: "message-send", + target: "email\\u{202E}system", + recipientCount: 3, + recipients: ["alice\\u{200B}@example.com", "bob@example.com"], + audience: "external", + }); + manager.resolve(record.id, "allow-once"); + await pending; + }); + + it("drops scope after escaped text exceeds its bounds without rejecting approval", async () => { + const { manager, handler, options } = createApprovalScopeRequest({ + kind: "external-post", + target: `github${"\u202E".repeat(20)}`, + visibility: "public", + }); + const pending = handler(options); + await vi.waitFor(() => expect(manager.listPendingRecords()).toHaveLength(1)); + const record = expectDefined(manager.listPendingRecords()[0], "pending plugin approval"); + + expect(record.request.scope).toBeNull(); + manager.resolve(record.id, "allow-once"); + await pending; + }); + + it.each([ + { kind: "untyped", target: "email" }, + { kind: "external-post", target: "github", visibility: "public", extra: true }, + ])("rejects malformed or non-closed owner-declared scope", async (scope) => { + const { manager, respond, handler, options } = createApprovalScopeRequest(scope); + await handler(options); + + expect(respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ code: expect.any(String) }), + ); + expect(manager.listPendingRecords()).toHaveLength(0); + }); +}); diff --git a/src/gateway/server-methods/plugin-approval.ts b/src/gateway/server-methods/plugin-approval.ts index 25a671ff455e..2e5a013b8b0c 100644 --- a/src/gateway/server-methods/plugin-approval.ts +++ b/src/gateway/server-methods/plugin-approval.ts @@ -7,11 +7,12 @@ import { validatePluginApprovalRequestParams, validatePluginApprovalResolveParams, } from "../../../packages/gateway-protocol/src/index.js"; +import { sanitizeApprovalScope, type ApprovalScope } from "../../infra/approval-scope.js"; +import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js"; import { sanitizeExecApprovalDisplayText, sanitizeExecApprovalWarningText, -} from "../../infra/exec-approval-command-display.js"; -import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js"; +} from "../../infra/exec-approval-text-sanitize.js"; import { resolveCanonicalPluginApprovalRequestAllowedDecisions } from "../../infra/plugin-approval-canonical-decisions.js"; import type { PluginApprovalRequest, @@ -88,6 +89,7 @@ export function createPluginApprovalHandlers( description: string; detail?: string | null; severity?: string | null; + scope?: ApprovalScope; toolName?: string | null; toolCallId?: string | null; allowedDecisions?: string[] | null; @@ -188,6 +190,7 @@ export function createPluginApprovalHandlers( pluginId: trustedAgentRuntime?.approvalOwnerPluginId ?? sanitizeMeta(p.pluginId), title: sanitizedTitle, description: sanitizedDescription, + scope: p.scope ? sanitizeApprovalScope(p.scope) : null, detail: rawDetail === null ? null diff --git a/src/infra/approval-presentation.test.ts b/src/infra/approval-presentation.test.ts index 872757838c32..495cdbad9e83 100644 --- a/src/infra/approval-presentation.test.ts +++ b/src/infra/approval-presentation.test.ts @@ -26,6 +26,40 @@ function buildPluginPresentation(request: { } describe("buildApprovalPresentation", () => { + it.each([ + { kind: "exec", request: { command: "printf safe" } }, + { + kind: "plugin", + request: { title: "Review payment", description: "The plugin needs operator consent." }, + }, + ] as const)("sanitizes $kind scope and drops scope that exceeds its wire bound", (params) => { + const scope = { + kind: "payment", + amount: "49.99", + currency: "EUR", + target: "Stripe\u202E", + } as const; + const presentation = buildApprovalPresentation({ + ...params, + request: { ...params.request, scope }, + allowedDecisions, + }); + + expect(presentation).toMatchObject({ + kind: params.kind, + scope: { ...scope, target: "Stripe\\u{202E}" }, + }); + + const oversizedScopePresentation = buildApprovalPresentation({ + ...params, + request: { ...params.request, scope: { ...scope, target: `${"x".repeat(125)}\u202E` } }, + allowedDecisions, + }); + + expect(oversizedScopePresentation).toMatchObject({ kind: params.kind }); + expect(oversizedScopePresentation).not.toHaveProperty("scope"); + }); + it("sanitizes exec routing metadata and preserves empty values as null", () => { const githubToken = `ghp_${"a".repeat(100)}`; const presentation = buildExecPresentation({ diff --git a/src/infra/approval-presentation.ts b/src/infra/approval-presentation.ts index d13fb8025470..ebd33d6ee240 100644 --- a/src/infra/approval-presentation.ts +++ b/src/infra/approval-presentation.ts @@ -7,11 +7,12 @@ import type { ApprovalKind, ApprovalPresentation, } from "../../packages/gateway-protocol/src/index.js"; +import { sanitizeApprovalScope } from "./approval-scope.js"; +import { resolveExecApprovalCommandDisplay } from "./exec-approval-command-display.js"; import { - resolveExecApprovalCommandDisplay, sanitizeExecApprovalDisplayText, sanitizeExecApprovalWarningText, -} from "./exec-approval-command-display.js"; +} from "./exec-approval-text-sanitize.js"; import type { ExecApprovalRequestPayload } from "./exec-approvals.js"; import { PLUGIN_APPROVAL_DESCRIPTION_MAX_LENGTH, @@ -59,6 +60,7 @@ function buildExecApprovalPresentation(params: { typeof request.warningText === "string" && request.warningText.trim() ? sanitizeExecApprovalWarningText(request.warningText) : null; + const scope = request.scope ? sanitizeApprovalScope(request.scope) : null; return { kind: "exec", commandText, @@ -67,6 +69,7 @@ function buildExecApprovalPresentation(params: { host: sanitizeOptionalSingleLine(request.host), nodeId: sanitizeOptionalSingleLine(request.nodeId), agentId: sanitizeOptionalSingleLine(request.agentId), + ...(scope ? { scope } : {}), allowedDecisions: normalizeDecisionList(params.allowedDecisions), }; } @@ -102,6 +105,7 @@ function buildPluginApprovalPresentation(params: { const detail = rawDetail ? truncatePluginApprovalDetail(sanitizeExecApprovalWarningText(rawDetail)) : null; + const scope = request.scope ? sanitizeApprovalScope(request.scope) : null; return { kind: "plugin", title, @@ -111,6 +115,7 @@ function buildPluginApprovalPresentation(params: { pluginId: sanitizeOptionalSingleLine(request.pluginId), toolName: sanitizeOptionalSingleLine(request.toolName), agentId: sanitizeOptionalSingleLine(request.agentId), + ...(scope ? { scope } : {}), allowedDecisions: normalizeDecisionList(params.allowedDecisions), }; } diff --git a/src/infra/approval-scope.test.ts b/src/infra/approval-scope.test.ts new file mode 100644 index 000000000000..ad6946393bb5 --- /dev/null +++ b/src/infra/approval-scope.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "vitest"; +import { + sanitizeApprovalScope, + summarizeApprovalScope, + type ApprovalScope, +} from "./approval-scope.js"; + +describe("approval scope", () => { + it.each([ + [ + { + kind: "message-send", + target: "email", + recipientCount: 3, + recipients: ["alice@x.com", "bob@y.com"], + audience: "external", + }, + "Send to 3 recipients via email (external): alice@x.com, bob@y.com, +1 more", + ], + [ + { kind: "message-send", target: "slack #general", recipientCount: 1 }, + "Send to 1 recipient via slack #general", + ], + [ + { kind: "payment", amount: "49.99", currency: "EUR", target: "Stripe" }, + "Pay 49.99 EUR to Stripe", + ], + [{ kind: "external-post", target: "github", visibility: "public" }, "Post publicly to github"], + [ + { kind: "external-post", target: "github", visibility: "restricted" }, + "Post restricted to github", + ], + ] satisfies [ApprovalScope, string][])("summarizes $kind", (scope, expected) => { + expect(summarizeApprovalScope(scope)).toBe(expected); + }); + + it("preserves clean scope fields and visibly escapes spoofing characters", () => { + const cleanScope = { + kind: "message-send", + target: "email", + recipientCount: 1, + recipients: ["alice@example.com"], + audience: "internal", + } satisfies ApprovalScope; + + expect(sanitizeApprovalScope(cleanScope)).toEqual(cleanScope); + expect(sanitizeApprovalScope({ ...cleanScope, target: "mail\u202Ebox" })).toMatchObject({ + target: "mail\\u{202E}box", + }); + expect(sanitizeApprovalScope({ ...cleanScope, target: "🦞".repeat(128) })).toMatchObject({ + target: "🦞".repeat(128), + }); + }); + + it("clamps recipient previews to the declared recipient count", () => { + const clamped = sanitizeApprovalScope({ + kind: "message-send", + target: "email", + recipientCount: 1, + recipients: ["alice@example.com", "bob@example.com"], + }); + expect(clamped).toMatchObject({ recipientCount: 1, recipients: ["alice@example.com"] }); + expect( + summarizeApprovalScope({ + kind: "message-send", + target: "email", + recipientCount: 1, + recipients: (clamped as Extract).recipients, + }), + ).toBe("Send to 1 recipient via email: alice@example.com"); + }); + + it.each([ + { kind: "message-send", target: `${"x".repeat(127)}\u202E`, recipientCount: 1 }, + { + kind: "message-send", + target: "email", + recipientCount: 1, + recipients: [`${"x".repeat(127)}\u202E`], + }, + { kind: "payment", amount: `${"x".repeat(39)}\u202E`, currency: "EUR", target: "Stripe" }, + { kind: "payment", amount: "49.99", currency: `${"x".repeat(11)}\u202E`, target: "Stripe" }, + { kind: "external-post", target: `${"x".repeat(127)}\u202E`, visibility: "public" }, + ] satisfies ApprovalScope[])( + "drops scopes that exceed a bound after escaping ($kind)", + (scope) => { + expect(sanitizeApprovalScope(scope)).toBeNull(); + }, + ); +}); diff --git a/src/infra/approval-scope.ts b/src/infra/approval-scope.ts new file mode 100644 index 000000000000..e815591c6677 --- /dev/null +++ b/src/infra/approval-scope.ts @@ -0,0 +1,63 @@ +import type { Static } from "typebox"; +import type { ApprovalScopeSchema } from "../../packages/gateway-protocol/src/schema/approvals.js"; +import { sanitizeExecApprovalDisplayText } from "./exec-approval-text-sanitize.js"; + +export type ApprovalScope = Static; + +function exceedsApprovalScopeStringLimit(value: string, maxLength: number): boolean { + return Array.from(value).length > maxLength; +} + +export function summarizeApprovalScope(scope: ApprovalScope): string { + switch (scope.kind) { + case "message-send": { + const recipientLabel = scope.recipientCount === 1 ? "recipient" : "recipients"; + const audience = scope.audience ? ` (${scope.audience})` : ""; + const recipients = scope.recipients ?? []; + const remaining = scope.recipientCount - recipients.length; + const preview = recipients.length + ? `: ${[...recipients, ...(remaining > 0 ? [`+${remaining} more`] : [])].join(", ")}` + : ""; + return `Send to ${scope.recipientCount} ${recipientLabel} via ${scope.target}${audience}${preview}`; + } + case "payment": + return `Pay ${scope.amount} ${scope.currency} to ${scope.target}`; + case "external-post": + return `Post ${scope.visibility === "public" ? "publicly" : "restricted"} to ${scope.target}`; + } + scope satisfies never; + throw new Error("Unsupported approval scope"); +} + +export function sanitizeApprovalScope(scope: ApprovalScope): ApprovalScope | null { + const target = sanitizeExecApprovalDisplayText(scope.target); + if (exceedsApprovalScopeStringLimit(target, 128)) { + return null; + } + + switch (scope.kind) { + case "message-send": { + // Previews are a subset of recipientCount; the count stays authoritative, + // so excess previews are clamped rather than rendered inconsistently. + const recipients = scope.recipients + ?.slice(0, scope.recipientCount) + .map(sanitizeExecApprovalDisplayText); + if (recipients?.some((recipient) => exceedsApprovalScopeStringLimit(recipient, 128))) { + return null; + } + return { ...scope, target, ...(recipients ? { recipients } : {}) }; + } + case "payment": { + const amount = sanitizeExecApprovalDisplayText(scope.amount); + const currency = sanitizeExecApprovalDisplayText(scope.currency); + return exceedsApprovalScopeStringLimit(amount, 40) || + exceedsApprovalScopeStringLimit(currency, 12) + ? null + : { ...scope, amount, currency, target }; + } + case "external-post": + return { ...scope, target }; + } + scope satisfies never; + return null; +} diff --git a/src/infra/approval-view-model.test.ts b/src/infra/approval-view-model.test.ts index 0c3b4101020d..46a2a0523f33 100644 --- a/src/infra/approval-view-model.test.ts +++ b/src/infra/approval-view-model.test.ts @@ -21,6 +21,13 @@ describe("buildPendingApprovalView", () => { riskKinds: ["inline-eval"], warningLines: ["Contains inline-eval: python -c"], }, + scope: { + kind: "message-send", + target: "email", + recipientCount: 3, + recipients: ["alice@example.com", "bob@example.com"], + audience: "external", + }, }, }; @@ -31,6 +38,12 @@ describe("buildPendingApprovalView", () => { throw new Error("expected exec approval view"); } expect(view.commandAnalysis?.warningLines).toEqual(["Contains inline-eval: python -c"]); + expect(view.scope).toEqual(request.request.scope); + expect(view.metadata).toContainEqual({ + label: "Scope", + value: + "Send to 3 recipients via email (external): alice@example.com, bob@example.com, +1 more", + }); expect(view.actions[0]?.action).toEqual({ type: "approval", approvalId: "approval-id", @@ -47,12 +60,15 @@ describe("buildPendingApprovalView", () => { request: { title: "Use protected tool", description: "The plugin needs operator consent.", + scope: { kind: "external-post", target: "github", visibility: "public" }, }, }; expect(resolveApprovalRequestKind(request)).toBe("plugin"); const view = buildPendingApprovalView(request); expect(view.approvalKind).toBe("plugin"); + expect(view.scope).toEqual(request.request.scope); + expect(view.metadata).toContainEqual({ label: "Scope", value: "Post publicly to github" }); expect(view.actions[0]?.action).toEqual({ type: "approval", approvalId: "custom-id-without-prefix", @@ -61,6 +77,24 @@ describe("buildPendingApprovalView", () => { }); }); + const approvalRequestBase = { id: "approval-id", createdAtMs: 1, expiresAtMs: 2 }; + + it.each([ + { request: { ...approvalRequestBase, request: { command: "echo safe" } }, metadata: [] }, + { + request: { + ...approvalRequestBase, + request: { title: "Use protected tool", description: "The plugin needs consent." }, + }, + metadata: [{ label: "Severity", value: "Warning" }], + }, + ])("preserves existing metadata when no approval scope is declared", ({ request, metadata }) => { + const view = buildPendingApprovalView(request); + + expect(view.metadata).toEqual(metadata); + expect(view).not.toHaveProperty("scope"); + }); + it("does not trust conflicting approval kind metadata", () => { const request: PluginApprovalRequest = { id: "plugin-approval", diff --git a/src/infra/approval-view-model.ts b/src/infra/approval-view-model.ts index 36271b52c885..6c4490a06734 100644 --- a/src/infra/approval-view-model.ts +++ b/src/infra/approval-view-model.ts @@ -1,4 +1,5 @@ // Builds approval prompt view models from request and resolution events. +import { summarizeApprovalScope } from "./approval-scope.js"; import { normalizeApprovalRequest } from "./approval-types.js"; import type { ApprovalMetadataView, @@ -35,6 +36,9 @@ function buildExecMetadata(request: ExecApprovalRequest): ApprovalMetadataView[] if (Array.isArray(request.request.envKeys) && request.request.envKeys.length > 0) { metadata.push({ label: "Env Overrides", value: request.request.envKeys.join(", ") }); } + if (request.request.scope) { + metadata.push({ label: "Scope", value: summarizeApprovalScope(request.request.scope) }); + } return metadata; } @@ -54,6 +58,9 @@ function buildPluginMetadata(request: PluginApprovalRequest): ApprovalMetadataVi if (request.request.agentId) { metadata.push({ label: "Agent", value: request.request.agentId }); } + if (request.request.scope) { + metadata.push({ label: "Scope", value: summarizeApprovalScope(request.request.scope) }); + } return metadata; } @@ -79,6 +86,7 @@ function buildExecViewBase( envKeys: request.request.envKeys ?? undefined, host: request.request.host ?? null, nodeId: request.request.nodeId ?? null, + ...(request.request.scope ? { scope: request.request.scope } : {}), sessionKey: request.request.sessionKey ?? null, }; } @@ -96,6 +104,7 @@ function buildPluginViewBase( metadata: buildPluginMetadata(request), agentId: request.request.agentId ?? null, pluginId: request.request.pluginId ?? null, + ...(request.request.scope ? { scope: request.request.scope } : {}), toolName: request.request.toolName ?? null, severity: request.request.severity ?? "warning", }; diff --git a/src/infra/approval-view-model.types.ts b/src/infra/approval-view-model.types.ts index 45e6f6b51150..a122d182b356 100644 --- a/src/infra/approval-view-model.types.ts +++ b/src/infra/approval-view-model.types.ts @@ -3,6 +3,7 @@ import type { MessagePresentationAction, MessagePresentationButton, } from "../interactive/payload.js"; +import type { ApprovalScope } from "./approval-scope.js"; import type { ApprovalRequestInput, ChannelApprovalKind } from "./approval-types.js"; import type { CommandExplanationSummary } from "./command-analysis/explain.js"; import type { ExecApprovalDecision, ExecApprovalResolved } from "./exec-approvals.js"; @@ -49,6 +50,7 @@ export type ExecApprovalViewBase = ApprovalViewBase & { envKeys?: readonly string[]; host?: string | null; nodeId?: string | null; + scope?: ApprovalScope | null; sessionKey?: string | null; }; @@ -76,6 +78,7 @@ export type PluginApprovalViewBase = ApprovalViewBase & { approvalKind: "plugin"; agentId?: string | null; pluginId?: string | null; + scope?: ApprovalScope | null; toolName?: string | null; severity: "info" | "warning" | "critical"; }; diff --git a/src/infra/exec-approval-command-display.test.ts b/src/infra/exec-approval-command-display.test.ts index 26252625be23..537af3503cde 100644 --- a/src/infra/exec-approval-command-display.test.ts +++ b/src/infra/exec-approval-command-display.test.ts @@ -1,10 +1,10 @@ // Verifies shell command display strings for exec approval prompts. import { describe, expect, it } from "vitest"; +import { resolveExecApprovalCommandDisplay } from "./exec-approval-command-display.js"; import { - resolveExecApprovalCommandDisplay, sanitizeExecApprovalDisplayText, sanitizeExecApprovalWarningText, -} from "./exec-approval-command-display.js"; +} from "./exec-approval-text-sanitize.js"; function hasLoneSurrogate(value: string): boolean { return Array.from(value).some((char) => { diff --git a/src/infra/exec-approval-command-display.ts b/src/infra/exec-approval-command-display.ts index 78796f6ad656..a171cee0d8c2 100644 --- a/src/infra/exec-approval-command-display.ts +++ b/src/infra/exec-approval-command-display.ts @@ -1,205 +1,7 @@ -import { expectDefined } from "@openclaw/normalization-core"; -import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; -// Sanitizes command text before it is displayed in approval prompts. -import { - computeSensitiveRedactionBitmap, - redactSensitiveText, - resolveRedactOptions, -} from "../logging/redact.js"; +// Resolves sanitized command/preview text for exec approval prompts. +import { sanitizeExecApprovalDisplayText } from "./exec-approval-text-sanitize.js"; import type { ExecApprovalRequestPayload } from "./exec-approvals.js"; -// Escape control characters, Unicode format/line/paragraph separators, unpaired surrogates, -// and non-ASCII space separators that can spoof or break approval prompts in common UIs. -// With the Unicode regex flag, valid astral characters are full code points and do not match -// Cs; only malformed surrogate code units are escaped. Ordinary ASCII space stays unchanged. -const EXEC_APPROVAL_INVISIBLE_CHAR_REGEX = - /[\p{Cc}\p{Cf}\p{Cs}\p{Zl}\p{Zp}\u00A0\u1680\u2000-\u200A\u202F\u205F\u3000\u115F\u1160\u3164\uFFA0]/gu; -const EXEC_APPROVAL_INVISIBLE_CHAR_SINGLE = - /^[\p{Cc}\p{Cf}\p{Cs}\p{Zl}\p{Zp}\u00A0\u1680\u2000-\u200A\u202F\u205F\u3000\u115F\u1160\u3164\uFFA0]$/u; - -// Hard cap on input the sanitizer will process at all. Above this size we return a constant -// marker without running any regex work, so an attacker cannot force unbounded CPU/memory. -const EXEC_APPROVAL_MAX_INPUT = 256 * 1024; -// Soft cap on displayed output. Truncation happens AFTER redaction so a secret near the -// cutoff is not partially exposed when the cut lands mid-token below a pattern's minimum -// length (e.g. `ghp_` needs 20+ trailing chars before the `\b` match). -const EXEC_APPROVAL_MAX_OUTPUT = 16 * 1024; -const EXEC_APPROVAL_TRUNCATION_MARKER = "…[truncated]"; -const EXEC_APPROVAL_OVERSIZED_MARKER = - "[exec approval command exceeds display size limit; full text suppressed]"; -const EXEC_APPROVAL_WARNING_OVERSIZED_MARKER = - "[exec approval warning exceeds display size limit; full text suppressed]"; - -const BYPASS_MASK = "***"; - -function formatCodePointEscape(char: string): string { - return `\\u{${char.codePointAt(0)?.toString(16).toUpperCase() ?? "FFFD"}}`; -} - -function normalizeDisplayLineBreaks(text: string): string { - return text.replace(/\r\n?/g, "\n").replace(/[\u2028\u2029]/g, "\n"); -} - -function escapeInvisibles(text: string, options?: { preserveLineBreaks?: boolean }): string { - return text.replace(EXEC_APPROVAL_INVISIBLE_CHAR_REGEX, (char) => - options?.preserveLineBreaks && char === "\n" ? "\n" : formatCodePointEscape(char), - ); -} - -/** Sanitized approval text plus size-cap status for callers that need UI affordances. */ -export type SanitizedExecApprovalDisplayText = { - /** Redacted, spoof-resistant command or warning text safe for an approval prompt. */ - text: string; - /** True when sanitized output exceeded the display cap and was shortened. */ - truncated: boolean; - /** True when raw input exceeded the hard cap and was replaced with a fixed marker. */ - oversized: boolean; -}; - -function truncateForDisplay(text: string): SanitizedExecApprovalDisplayText { - if (text.length <= EXEC_APPROVAL_MAX_OUTPUT) { - return { text, truncated: false, oversized: false }; - } - return { - text: truncateUtf16Safe(text, EXEC_APPROVAL_MAX_OUTPUT) + EXEC_APPROVAL_TRUNCATION_MARKER, - truncated: true, - oversized: false, - }; -} - -// Iterate by full Unicode code point so astral-plane invisibles (e.g. U+E0061 TAG LATIN -// SMALL LETTER A, category Cf) are matched as single characters instead of being seen as a -// surrogate pair whose halves are category Cs and would escape the invisible-char regex. -function buildStrippedView(original: string): { stripped: string; strippedToOrig: number[] } { - const strippedChars: string[] = []; - const strippedToOrig: number[] = []; - let offset = 0; - for (const cp of original) { - if (!EXEC_APPROVAL_INVISIBLE_CHAR_SINGLE.test(cp)) { - strippedChars.push(cp); - for (let k = 0; k < cp.length; k++) { - strippedToOrig.push(offset + k); - } - } - offset += cp.length; - } - return { stripped: strippedChars.join(""), strippedToOrig }; -} - -function sanitizeExecApprovalDisplayTextInternal( - commandText: string, - options?: { preserveLineBreaks?: boolean; oversizedMarker?: string }, -): SanitizedExecApprovalDisplayText { - if (commandText.length > EXEC_APPROVAL_MAX_INPUT) { - // Refuse to display inputs above the hard cap; anything larger must be approved through - // another channel. Running redaction on a multi-megabyte payload would be a DoS vector. - return { - text: options?.oversizedMarker ?? EXEC_APPROVAL_OVERSIZED_MARKER, - truncated: false, - oversized: true, - }; - } - const rawRedacted = redactSensitiveText(commandText, { mode: "tools" }); - const { stripped, strippedToOrig } = buildStrippedView(commandText); - const strippedRedacted = redactSensitiveText(stripped, { mode: "tools" }); - // Fast path: stripping invisibles did not expose any additional secret-like content, so the - // raw-view redaction is sufficient. Preserve structure and show invisible-character spoof - // attempts as `\u{...}` escapes. - if (strippedRedacted === stripped) { - return truncateForDisplay(escapeInvisibles(rawRedacted, options)); - } - // Detect bypass by position-bitmap coverage. Run the redaction matchers on both views and - // map stripped-view match positions back to original coordinates. If every position the - // stripped view would mask is also masked by the raw view, the raw view already covered - // everything — for example, an ordinary multi-line PEM private key where raw produces - // `BEGIN/…redacted…/END` while stripped collapses to `***`. A real bypass exists only when - // the stripped view masks at least one original position raw missed (e.g. the tail of an - // `sk-` token whose prefix-boundary was broken by a spliced zero-width or NBSP character). - const redaction = resolveRedactOptions({ mode: "tools" }); - const rawMask = computeSensitiveRedactionBitmap(commandText, redaction); - const strippedMask = computeSensitiveRedactionBitmap(stripped, redaction); - let bypassDetected = false; - for (let i = 0; i < strippedMask.length; i++) { - if ( - strippedMask[i] && - !rawMask[expectDefined(strippedToOrig[i], "stripped to orig entry at i")] - ) { - bypassDetected = true; - break; - } - } - if (!bypassDetected) { - return truncateForDisplay(escapeInvisibles(rawRedacted, options)); - } - // Bypass path. Project the stripped-view mask back onto original positions, union with the - // raw-view mask, and emit a rendering where each contiguous masked run becomes a single - // `***` marker. Invisible characters that fall outside masked runs still render as visible - // `\u{...}` escapes so multi-line structure and spliced invisibles stay readable. The - // render loop advances by full code point so astral-plane invisibles are escaped as one - // `\u{...}` token rather than two separate surrogate escapes (or, worse, passed through - // unescaped because neither surrogate half matches the Cf regex). - const unionMask = rawMask.slice(); - for (let i = 0; i < strippedMask.length; i++) { - if (strippedMask[i]) { - unionMask[expectDefined(strippedToOrig[i], "stripped to orig entry at i")] = true; - } - } - let out = ""; - let i = 0; - while (i < commandText.length) { - if (unionMask[i]) { - let j = i; - while (j < commandText.length && unionMask[j]) { - j++; - } - out += BYPASS_MASK; - i = j; - continue; - } - const codePoint = commandText.codePointAt(i) ?? 0xfffd; - const cp = String.fromCodePoint(codePoint); - out += - options?.preserveLineBreaks && cp === "\n" - ? cp - : EXEC_APPROVAL_INVISIBLE_CHAR_SINGLE.test(cp) - ? formatCodePointEscape(cp) - : cp; - i += cp.length; - } - return truncateForDisplay(out); -} - -/** Sanitizes exec command text for approval UI without exposing status metadata. */ -export function sanitizeExecApprovalDisplayText(commandText: string): string { - return sanitizeExecApprovalDisplayTextInternal(commandText).text; -} - -/** - * Sanitizes exec command text for approval UI and reports whether size caps changed it. - */ -export function sanitizeExecApprovalDisplayTextWithStatus( - commandText: string, -): SanitizedExecApprovalDisplayText { - return sanitizeExecApprovalDisplayTextInternal(commandText); -} - -/** - * Sanitizes warning prose for approval UI while preserving real line boundaries. - */ -export function sanitizeExecApprovalWarningText(warningText: string): string { - return sanitizeExecApprovalWarningTextWithStatus(warningText).text; -} - -/** Sanitizes warning prose and reports whether display bounds suppressed any content. */ -export function sanitizeExecApprovalWarningTextWithStatus( - warningText: string, -): SanitizedExecApprovalDisplayText { - return sanitizeExecApprovalDisplayTextInternal(normalizeDisplayLineBreaks(warningText), { - preserveLineBreaks: true, - oversizedMarker: EXEC_APPROVAL_WARNING_OVERSIZED_MARKER, - }); -} - function normalizePreview(commandText: string, commandPreview?: string | null): string | null { const previewRaw = commandPreview?.trim() ?? ""; if (!previewRaw) { diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index 4b8ded085943..011a571e6481 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -26,11 +26,9 @@ import { createPendingApprovalRegistry } from "../shared/pending-approval-regist import { isDeliverableMessageChannel, normalizeMessageChannel } from "../utils/message-channel.js"; import { matchesApprovalRequestFilters } from "./approval-request-filters.js"; import type { ChannelApprovalKind } from "./approval-types.js"; -import { - resolveExecApprovalCommandDisplay, - sanitizeExecApprovalWarningText, -} from "./exec-approval-command-display.js"; +import { resolveExecApprovalCommandDisplay } from "./exec-approval-command-display.js"; import { formatExecApprovalExpiresIn } from "./exec-approval-reply.js"; +import { sanitizeExecApprovalWarningText } from "./exec-approval-text-sanitize.js"; import { resolveExecApprovalRequestAllowedDecisions, type ExecApprovalRequest, diff --git a/src/infra/exec-approval-reply.test.ts b/src/infra/exec-approval-reply.test.ts index 69bac1be7042..5aede9ce1ca4 100644 --- a/src/infra/exec-approval-reply.test.ts +++ b/src/infra/exec-approval-reply.test.ts @@ -267,6 +267,7 @@ describe("exec approval reply helpers", () => { cwd: "/tmp/work", host: "gateway", nodeId: "node-1", + scope: { kind: "payment", amount: "49.99", currency: "EUR", target: "Stripe" }, expiresAtMs: 2500, nowMs: 1000, }); @@ -324,7 +325,9 @@ describe("exec approval reply helpers", () => { expect(payload.text).toContain("Heads up."); expect(payload.text).toContain("```txt\n/approve slug-1 allow-once\n```"); expect(payload.text).toContain("```sh\necho ok\n```"); - expect(payload.text).toContain("Host: gateway\nNode: node-1\nCWD: /tmp/work\nExpires in: 2s"); + expect(payload.text).toContain( + "Host: gateway\nNode: node-1\nCWD: /tmp/work\nScope: Pay 49.99 EUR to Stripe\nExpires in: 2s", + ); expect(payload.text).toContain("Full id: `req-1`"); }); @@ -352,6 +355,7 @@ describe("exec approval reply helpers", () => { }, ], }); + expect(payload.text).not.toContain("Scope:"); }); it("compacts structured cwd paths in pending reply payloads", () => { diff --git a/src/infra/exec-approval-reply.ts b/src/infra/exec-approval-reply.ts index 2033cb1b32cf..12d5e2f21da8 100644 --- a/src/infra/exec-approval-reply.ts +++ b/src/infra/exec-approval-reply.ts @@ -14,6 +14,7 @@ import { formatHumanList } from "../shared/human-list.js"; // Builds reply payloads for exec approval prompts and outcomes. import { formatFencedCodeBlock } from "../shared/markdown-code.js"; import { formatApprovalDisplayPath } from "./approval-display-paths.js"; +import { summarizeApprovalScope, type ApprovalScope } from "./approval-scope.js"; import type { ChannelApprovalKind } from "./approval-types.js"; import { describeNativeExecApprovalClientSetup, @@ -68,6 +69,7 @@ export type ExecApprovalPendingReplyParams = { cwd?: string; host: ExecHost; nodeId?: string; + scope?: ApprovalScope | null; sessionKey?: string | null; expiresAtMs?: number; nowMs?: number; @@ -431,6 +433,9 @@ export function buildExecApprovalPendingReplyPayload( if (params.cwd) { info.push(`CWD: ${formatApprovalDisplayPath(params.cwd)}`); } + if (params.scope) { + info.push(`Scope: ${summarizeApprovalScope(params.scope)}`); + } if (typeof params.expiresAtMs === "number" && Number.isFinite(params.expiresAtMs)) { info.push( `Expires in: ${formatExecApprovalExpiresIn(params.expiresAtMs, params.nowMs ?? Date.now())}`, diff --git a/src/infra/exec-approval-text-sanitize.ts b/src/infra/exec-approval-text-sanitize.ts new file mode 100644 index 000000000000..e94837642ad7 --- /dev/null +++ b/src/infra/exec-approval-text-sanitize.ts @@ -0,0 +1,201 @@ +import { expectDefined } from "@openclaw/normalization-core"; +import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; +// Leaf sanitizer for approval display text; keep free of exec-approvals imports +// so approval-scope and exec-approvals-config can share it without a cycle. +import { + computeSensitiveRedactionBitmap, + redactSensitiveText, + resolveRedactOptions, +} from "../logging/redact.js"; + +// Escape control characters, Unicode format/line/paragraph separators, unpaired surrogates, +// and non-ASCII space separators that can spoof or break approval prompts in common UIs. +// With the Unicode regex flag, valid astral characters are full code points and do not match +// Cs; only malformed surrogate code units are escaped. Ordinary ASCII space stays unchanged. +const EXEC_APPROVAL_INVISIBLE_CHAR_REGEX = + /[\p{Cc}\p{Cf}\p{Cs}\p{Zl}\p{Zp}\u00A0\u1680\u2000-\u200A\u202F\u205F\u3000\u115F\u1160\u3164\uFFA0]/gu; +const EXEC_APPROVAL_INVISIBLE_CHAR_SINGLE = + /^[\p{Cc}\p{Cf}\p{Cs}\p{Zl}\p{Zp}\u00A0\u1680\u2000-\u200A\u202F\u205F\u3000\u115F\u1160\u3164\uFFA0]$/u; + +// Hard cap on input the sanitizer will process at all. Above this size we return a constant +// marker without running any regex work, so an attacker cannot force unbounded CPU/memory. +const EXEC_APPROVAL_MAX_INPUT = 256 * 1024; +// Soft cap on displayed output. Truncation happens AFTER redaction so a secret near the +// cutoff is not partially exposed when the cut lands mid-token below a pattern's minimum +// length (e.g. `ghp_` needs 20+ trailing chars before the `\b` match). +const EXEC_APPROVAL_MAX_OUTPUT = 16 * 1024; +const EXEC_APPROVAL_TRUNCATION_MARKER = "…[truncated]"; +const EXEC_APPROVAL_OVERSIZED_MARKER = + "[exec approval command exceeds display size limit; full text suppressed]"; +const EXEC_APPROVAL_WARNING_OVERSIZED_MARKER = + "[exec approval warning exceeds display size limit; full text suppressed]"; + +const BYPASS_MASK = "***"; + +function formatCodePointEscape(char: string): string { + return `\\u{${char.codePointAt(0)?.toString(16).toUpperCase() ?? "FFFD"}}`; +} + +function normalizeDisplayLineBreaks(text: string): string { + return text.replace(/\r\n?/g, "\n").replace(/[\u2028\u2029]/g, "\n"); +} + +function escapeInvisibles(text: string, options?: { preserveLineBreaks?: boolean }): string { + return text.replace(EXEC_APPROVAL_INVISIBLE_CHAR_REGEX, (char) => + options?.preserveLineBreaks && char === "\n" ? "\n" : formatCodePointEscape(char), + ); +} + +/** Sanitized approval text plus size-cap status for callers that need UI affordances. */ +export type SanitizedExecApprovalDisplayText = { + /** Redacted, spoof-resistant command or warning text safe for an approval prompt. */ + text: string; + /** True when sanitized output exceeded the display cap and was shortened. */ + truncated: boolean; + /** True when raw input exceeded the hard cap and was replaced with a fixed marker. */ + oversized: boolean; +}; + +function truncateForDisplay(text: string): SanitizedExecApprovalDisplayText { + if (text.length <= EXEC_APPROVAL_MAX_OUTPUT) { + return { text, truncated: false, oversized: false }; + } + return { + text: truncateUtf16Safe(text, EXEC_APPROVAL_MAX_OUTPUT) + EXEC_APPROVAL_TRUNCATION_MARKER, + truncated: true, + oversized: false, + }; +} + +// Iterate by full Unicode code point so astral-plane invisibles (e.g. U+E0061 TAG LATIN +// SMALL LETTER A, category Cf) are matched as single characters instead of being seen as a +// surrogate pair whose halves are category Cs and would escape the invisible-char regex. +function buildStrippedView(original: string): { stripped: string; strippedToOrig: number[] } { + const strippedChars: string[] = []; + const strippedToOrig: number[] = []; + let offset = 0; + for (const cp of original) { + if (!EXEC_APPROVAL_INVISIBLE_CHAR_SINGLE.test(cp)) { + strippedChars.push(cp); + for (let k = 0; k < cp.length; k++) { + strippedToOrig.push(offset + k); + } + } + offset += cp.length; + } + return { stripped: strippedChars.join(""), strippedToOrig }; +} + +function sanitizeExecApprovalDisplayTextInternal( + commandText: string, + options?: { preserveLineBreaks?: boolean; oversizedMarker?: string }, +): SanitizedExecApprovalDisplayText { + if (commandText.length > EXEC_APPROVAL_MAX_INPUT) { + // Refuse to display inputs above the hard cap; anything larger must be approved through + // another channel. Running redaction on a multi-megabyte payload would be a DoS vector. + return { + text: options?.oversizedMarker ?? EXEC_APPROVAL_OVERSIZED_MARKER, + truncated: false, + oversized: true, + }; + } + const rawRedacted = redactSensitiveText(commandText, { mode: "tools" }); + const { stripped, strippedToOrig } = buildStrippedView(commandText); + const strippedRedacted = redactSensitiveText(stripped, { mode: "tools" }); + // Fast path: stripping invisibles did not expose any additional secret-like content, so the + // raw-view redaction is sufficient. Preserve structure and show invisible-character spoof + // attempts as `\u{...}` escapes. + if (strippedRedacted === stripped) { + return truncateForDisplay(escapeInvisibles(rawRedacted, options)); + } + // Detect bypass by position-bitmap coverage. Run the redaction matchers on both views and + // map stripped-view match positions back to original coordinates. If every position the + // stripped view would mask is also masked by the raw view, the raw view already covered + // everything — for example, an ordinary multi-line PEM private key where raw produces + // `BEGIN/…redacted…/END` while stripped collapses to `***`. A real bypass exists only when + // the stripped view masks at least one original position raw missed (e.g. the tail of an + // `sk-` token whose prefix-boundary was broken by a spliced zero-width or NBSP character). + const redaction = resolveRedactOptions({ mode: "tools" }); + const rawMask = computeSensitiveRedactionBitmap(commandText, redaction); + const strippedMask = computeSensitiveRedactionBitmap(stripped, redaction); + let bypassDetected = false; + for (let i = 0; i < strippedMask.length; i++) { + if ( + strippedMask[i] && + !rawMask[expectDefined(strippedToOrig[i], "stripped to orig entry at i")] + ) { + bypassDetected = true; + break; + } + } + if (!bypassDetected) { + return truncateForDisplay(escapeInvisibles(rawRedacted, options)); + } + // Bypass path. Project the stripped-view mask back onto original positions, union with the + // raw-view mask, and emit a rendering where each contiguous masked run becomes a single + // `***` marker. Invisible characters that fall outside masked runs still render as visible + // `\u{...}` escapes so multi-line structure and spliced invisibles stay readable. The + // render loop advances by full code point so astral-plane invisibles are escaped as one + // `\u{...}` token rather than two separate surrogate escapes (or, worse, passed through + // unescaped because neither surrogate half matches the Cf regex). + const unionMask = rawMask.slice(); + for (let i = 0; i < strippedMask.length; i++) { + if (strippedMask[i]) { + unionMask[expectDefined(strippedToOrig[i], "stripped to orig entry at i")] = true; + } + } + let out = ""; + let i = 0; + while (i < commandText.length) { + if (unionMask[i]) { + let j = i; + while (j < commandText.length && unionMask[j]) { + j++; + } + out += BYPASS_MASK; + i = j; + continue; + } + const codePoint = commandText.codePointAt(i) ?? 0xfffd; + const cp = String.fromCodePoint(codePoint); + out += + options?.preserveLineBreaks && cp === "\n" + ? cp + : EXEC_APPROVAL_INVISIBLE_CHAR_SINGLE.test(cp) + ? formatCodePointEscape(cp) + : cp; + i += cp.length; + } + return truncateForDisplay(out); +} + +/** Sanitizes exec command text for approval UI without exposing status metadata. */ +export function sanitizeExecApprovalDisplayText(commandText: string): string { + return sanitizeExecApprovalDisplayTextInternal(commandText).text; +} + +/** + * Sanitizes exec command text for approval UI and reports whether size caps changed it. + */ +export function sanitizeExecApprovalDisplayTextWithStatus( + commandText: string, +): SanitizedExecApprovalDisplayText { + return sanitizeExecApprovalDisplayTextInternal(commandText); +} + +/** + * Sanitizes warning prose for approval UI while preserving real line boundaries. + */ +export function sanitizeExecApprovalWarningText(warningText: string): string { + return sanitizeExecApprovalWarningTextWithStatus(warningText).text; +} + +/** Sanitizes warning prose and reports whether display bounds suppressed any content. */ +export function sanitizeExecApprovalWarningTextWithStatus( + warningText: string, +): SanitizedExecApprovalDisplayText { + return sanitizeExecApprovalDisplayTextInternal(normalizeDisplayLineBreaks(warningText), { + preserveLineBreaks: true, + oversizedMarker: EXEC_APPROVAL_WARNING_OVERSIZED_MARKER, + }); +} diff --git a/src/infra/exec-approvals-core.ts b/src/infra/exec-approvals-core.ts index 65eaacc1ff7b..ccf623fe4114 100644 --- a/src/infra/exec-approvals-core.ts +++ b/src/infra/exec-approvals-core.ts @@ -1,5 +1,6 @@ // Shared exec approval types and mode normalization. import { normalizeOptionalLowercaseString } from "@openclaw/normalization-core/string-coerce"; +import type { ApprovalScope } from "./approval-scope.js"; import type { CommandExplanationSummary } from "./command-analysis/explain.js"; import type { ExecApprovalPolicySnapshot } from "./exec-approval-policy-snapshot.js"; import type { ExecAllowlistEntry } from "./exec-approvals.types.js"; @@ -195,6 +196,8 @@ export type ExecApprovalRequestPayload = { security?: string | null; ask?: string | null; warningText?: string | null; + /** Owner-declared blast-radius facts; display-only, never authorization. */ + scope?: ApprovalScope | null; commandAnalysis?: CommandExplanationSummary | null; commandSpans?: ExecApprovalCommandSpan[]; unavailableDecisions?: readonly ExecApprovalUnavailableDecision[]; diff --git a/src/infra/plugin-approvals.ts b/src/infra/plugin-approvals.ts index 3c2746269989..ec7697edcb86 100644 --- a/src/infra/plugin-approvals.ts +++ b/src/infra/plugin-approvals.ts @@ -1,5 +1,6 @@ // Defines plugin approval request/resolution payloads and actions. import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; +import { summarizeApprovalScope, type ApprovalScope } from "./approval-scope.js"; import type { ExecApprovalDecision } from "./exec-approvals.js"; // Plugin approval types and renderers mirror exec approval decisions while @@ -20,6 +21,8 @@ export type PluginApprovalRequestPayload = { description: string; detail?: string | null; severity?: "info" | "warning" | "critical" | null; + /** Owner-declared blast-radius facts; display-only, never authorization. */ + scope?: ApprovalScope | null; toolName?: string | null; toolCallId?: string | null; allowedDecisions?: readonly ExecApprovalDecision[] | null; @@ -133,6 +136,9 @@ export function buildPluginApprovalRequestMessage( lines.push(`Title: ${request.request.title}`); // Reviewer-only detail stays off channel messages; channels receive the bounded description. lines.push(`Description: ${request.request.description}`); + if (request.request.scope) { + lines.push(`Scope: ${summarizeApprovalScope(request.request.scope)}`); + } if (request.request.toolName) { lines.push(`Tool: ${request.request.toolName}`); } diff --git a/src/plugin-sdk/approval-reaction-runtime.test.ts b/src/plugin-sdk/approval-reaction-runtime.test.ts index 6b2084bfde62..f6e44cb7e874 100644 --- a/src/plugin-sdk/approval-reaction-runtime.test.ts +++ b/src/plugin-sdk/approval-reaction-runtime.test.ts @@ -217,13 +217,22 @@ describe("plugin-sdk/approval-reaction-runtime", () => { }); it("builds canonical exec reaction prompts without presentation controls", () => { - const payload = buildApprovalReactionPromptPayloadForRequest({ - request: execRequest, + const content = buildApprovalReactionPendingContentForRequest({ + request: { + ...execRequest, + request: { + ...execRequest.request, + scope: { kind: "payment", amount: "49.99", currency: "EUR", target: "Stripe" }, + }, + }, nowMs: 1_000, }); + const payload = content.reactionPayload; expect(payload.text).toContain("**Exec approval required**\n**ID:** exec-approval-123"); expect(payload.text).toContain("**Pending command:**\n```sh\ntouch /tmp/foo\n```"); + expect(payload.text).toContain("**Scope:** Pay 49.99 EUR to Stripe"); + expect(content.manualFallbackPayload.text).toContain("Scope: Pay 49.99 EUR to Stripe"); expect(payload.text).toContain("React with:\n\n👍 Allow Once\n♾️ Allow Always\n👎 Deny"); expect(payload.text).toContain("Allow Once: /approve exec-approval-123 allow-once"); expect(payload.text).toContain("Allow Always: /approve exec-approval-123 allow-always"); @@ -287,6 +296,7 @@ describe("plugin-sdk/approval-reaction-runtime", () => { request: { ...pluginRequest.request, allowedDecisions: ["allow-once", "deny"], + scope: { kind: "external-post", target: "github", visibility: "public" }, }, }, nowMs: 1_000, @@ -294,6 +304,7 @@ describe("plugin-sdk/approval-reaction-runtime", () => { expect(payload.text).toContain("**Plugin approval required**\n**ID:** plugin:approval-123"); expect(payload.text).toContain("**Title:** Use 1Password"); + expect(payload.text).toContain("**Scope:** Post publicly to github"); expect(payload.text).toContain("React with:\n\n👍 Allow Once\n👎 Deny"); expect(payload.text).not.toContain("♾️ Allow Always"); expect(payload.text).toContain("Allow Once: /approve plugin:approval-123 allow-once"); diff --git a/src/plugin-sdk/approval-reaction-runtime.ts b/src/plugin-sdk/approval-reaction-runtime.ts index 10fa983f564f..509540a04581 100644 --- a/src/plugin-sdk/approval-reaction-runtime.ts +++ b/src/plugin-sdk/approval-reaction-runtime.ts @@ -1,5 +1,6 @@ import { sanitizeForPromptLiteral } from "../agents/sanitize-for-prompt.js"; import { formatApprovalDisplayPath } from "../infra/approval-display-paths.js"; +import { summarizeApprovalScope } from "../infra/approval-scope.js"; import { normalizeApprovalRequest, type ChannelApprovalKind } from "../infra/approval-types.js"; import { buildPendingApprovalView } from "../infra/approval-view-model.js"; import type { ApprovalRequest, PendingApprovalView } from "../infra/approval-view-model.types.js"; @@ -308,6 +309,7 @@ function buildApprovalReactionPromptText(params: { reactionHint: string | null; }): string { const { view } = params; + const scopeSummary = view.scope ? summarizeApprovalScope(view.scope) : undefined; const allowedDecisions = listDecisionActions(view.actions); const sections: string[] = []; if (view.approvalKind === "exec") { @@ -347,6 +349,9 @@ function buildApprovalReactionPromptText(params: { if (view.nodeId) { info.push(`**Node:** ${view.nodeId}`); } + if (scopeSummary) { + info.push(`**Scope:** ${scopeSummary}`); + } if (view.agentId) { info.push(`**Agent:** ${view.agentId}`); } @@ -363,6 +368,9 @@ function buildApprovalReactionPromptText(params: { if (view.description) { details.push(`**Description:** ${view.description}`); } + if (scopeSummary) { + details.push(`**Scope:** ${scopeSummary}`); + } details.push(`**Severity:** ${formatSeverity(view.severity)}`); if (view.toolName) { details.push(`**Tool:** ${view.toolName}`); @@ -504,6 +512,7 @@ export function buildApprovalReactionPendingContent(params: { cwd: params.view.cwd ?? undefined, host: params.view.host === "node" ? "node" : "gateway", nodeId: params.view.nodeId ?? undefined, + scope: params.view.scope ?? undefined, sessionKey: params.view.sessionKey ?? null, expiresAtMs: request.expiresAtMs, nowMs: params.nowMs, diff --git a/src/plugin-sdk/approval-runtime.ts b/src/plugin-sdk/approval-runtime.ts index b1448ff2ac3b..ef3962e8f411 100644 --- a/src/plugin-sdk/approval-runtime.ts +++ b/src/plugin-sdk/approval-runtime.ts @@ -1,5 +1,6 @@ // Approval request/reply helpers for exec and plugin approval flows. +export type { ApprovalScope } from "../infra/approval-scope.js"; export { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS, resolveExecApprovalAllowedDecisions, diff --git a/src/plugins/hook-before-tool-call-result.ts b/src/plugins/hook-before-tool-call-result.ts index d244f7d68382..701ec00d72c4 100644 --- a/src/plugins/hook-before-tool-call-result.ts +++ b/src/plugins/hook-before-tool-call-result.ts @@ -1,3 +1,5 @@ +import type { ApprovalScope } from "../infra/approval-scope.js"; + export const PluginApprovalResolutions = { ALLOW_ONCE: "allow-once", ALLOW_ALWAYS: "allow-always", @@ -16,6 +18,7 @@ export type PluginHookBeforeToolCallResult = { requireApproval?: { title: string; description: string; + scope?: ApprovalScope; severity?: "info" | "warning" | "critical"; timeoutMs?: number; /** diff --git a/src/plugins/plugin-registration.types.ts b/src/plugins/plugin-registration.types.ts index 1dc4df6b6560..148fc8e9bbb7 100644 --- a/src/plugins/plugin-registration.types.ts +++ b/src/plugins/plugin-registration.types.ts @@ -4,6 +4,7 @@ import type { Result } from "@openclaw/normalization-core/result"; import type { Command } from "commander"; import type { MessageReceipt } from "../channels/message/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import type { ApprovalScope } from "../infra/approval-scope.js"; import type { DiagnosticEventPrivateData, DiagnosticEventInput, @@ -221,6 +222,7 @@ type OpenClawPluginNodeInvokePolicyApprovalRuntime = { request: (input: { title: string; description: string; + scope?: ApprovalScope; severity?: "info" | "warning" | "critical"; toolName?: string; toolCallId?: string;